Re: Compiling: error: cast increases required alignment of target type [-Werror=cast-align]
Hi Denis, >>> On 02/04/2019 04:28 PM, Pičugins Arsenijs wrote: CC drivers/mbimmodem/mbim-message.o drivers/mbimmodem/mbim-message.c: In function ‘_iter_copy_string’: drivers/mbimmodem/mbim-message.c:199:18: error: cast increases required alignment of target type [-Werror=cast-align] uint16_t *le = (uint16_t *) buf; ^ >>> >>> Does the attached patch fix this? >> It fixes it for me. Please apply. >> // Martin > > All right, applied. I wonder if using l_get_u16 here to ensure that we always get the alignment right isn’t better. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH 2/2] udevng: Catch network interface renames
Hi Pau,
> If for whatever reason a network interface belonging to a modem is
> renamed after ofono startup, ofono then keeps advertising the old
> non-existing interface name instead of the new one.
> To catch the changes we use udev's "move" action together with
> DEVPATH_OLD variable, which points to the old devpath of the device.
>
> Unfortunately current debian9 systemd version contains systemd bug which
> prevents this feature to work fine if the iface is renamed more than
> once, see [1] for more information.
>
> [1] https://github.com/systemd/systemd/issues/9426
> ---
> plugins/udevng.c | 84 +++-
> 1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index c518553a..8338927b 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -56,6 +56,7 @@ struct modem_info {
> };
> struct ofono_modem *modem;
> const char *sysattr;
> + gboolean needs_update;
> };
>
> struct device_info {
> @@ -1630,6 +1631,7 @@ static void add_device(const char *syspath, const char
> *devname,
>
> modem->devices = g_slist_insert_sorted(modem->devices, info,
> compare_device);
> + modem->needs_update = TRUE;
> }
>
> static struct {
> @@ -1822,6 +1824,8 @@ static gboolean create_modem(gpointer key, gpointer
> value, gpointer user_data)
> const char *syspath = key;
> unsigned int i;
>
> + modem->needs_update = FALSE;
> +
> if (modem->modem != NULL)
> return FALSE;
>
> @@ -1909,6 +1913,81 @@ static gboolean check_modem_list(gpointer user_data)
> return FALSE;
> }
>
> +static void drop_old_device(gpointer key, gpointer value, gpointer user_data)
> +{
> + const char *syspath = key;
> + struct modem_info *modem = value;
> + struct udev_device *device = user_data;
> + const char *devpath_old;
> + GSList *list;
> +
> + devpath_old = udev_device_get_property_value(device, "DEVPATH_OLD");
> + if (!devpath_old)
> + return;
> +
> + for (list = modem->devices; list; list = list->next) {
> + struct device_info *info = list->data;
> + struct udev_device *dev;
> +
> + if (g_strcmp0(info->subsystem, "net") != 0)
> + continue;
> +
> + if (g_strcmp0(info->devpath, devpath_old) != 0)
> + continue;
> +
> + /* Drop no longer existing net iface device */
> + DBG("Dropping lost device %s", info->devpath);
I really prefer that DBG is surrounded by empty lines.
> + modem->devices = g_slist_remove_link(modem->devices, list);
> + g_slist_free_1 (list);
No space between 1 and ( here.
> + device_info_free(info);
> + udev_device_unref(dev);
> + modem->needs_update = TRUE;
> + }
> +}
> +
> +
> +static void update_modem(gpointer key, gpointer value, gpointer user_data)
> +{
> + const char *syspath = key;
> + struct modem_info *modem = value;
> + unsigned int i;
> +
> + if (!modem->needs_update)
> + return;
> +
> + DBG("Updating modem %s", syspath);
> +
> + for (i = 0; driver_list[i].name; i++) {
> + if (g_str_equal(driver_list[i].name, modem->driver) == FALSE)
> + continue;
> + if (driver_list[i].setup(modem) == FALSE)
> + continue;
> + }
> +}
> +static void update_device(struct udev_device *device)
> +{
> + const char *syspath;
> +
> + syspath = udev_device_get_syspath(device);
> + if (syspath == NULL)
> + return;
> +
> + DBG("%s", syspath);
> +
> + /* We only care about net iface renames for now */
> + if (g_strcmp0(udev_device_get_subsystem(device), "net") != 0)
> + return;
> +
> + /* Drop old device from modem */
> + g_hash_table_foreach(modem_list, drop_old_device, device);
> +
> + /* Add new device to modem */
> + check_device(device);
> +
> + /* Update modified modems */
> + g_hash_table_foreach(modem_list, update_modem, NULL);
> +}
> +
> static gboolean udev_event(GIOChannel *channel, GIOCondition cond,
> gpointer user_data)
> {
> @@ -1936,8 +2015,11 @@ static gboolean udev_event(GIOChannel *channel,
> GIOCondition cond,
> check_device(device);
>
> udev_delay = g_timeout_add_seconds(1, check_modem_list, NULL);
> - } else if (g_str_equal(action, "remove") == TRUE)
> + } else if (g_str_equal(action, "remove") == TRUE) {
> remove_device(device);
> + } else if (g_str_equal(action, "move") == TRUE) {
> + update_device(device);
> + }
>
> udev_device_unref(device);
Wouldn’t it generally be better for reading the name after oFono has brought up
the interface. In case we need this for the D-Bus API som
Re: [PATCH] document getting of ell, point people at useful docs
Hi Pavel, >>> diff --git a/README b/README >>> index 45bb2e9..04a50a5 100644 >>> --- a/README >>> +++ b/README >>> @@ -16,11 +16,14 @@ To configure run: >>> ./configure --prefix=/usr --mandir=/usr/share/man \ >>> --sysconfdir=/etc --localstatedir=/var >>> >>> -Configure automatically searches for all required components and packages. >>> +Configure automatically searches for all required components and >>> +packages, except ell, mentioned below. >>> >> >> Nope. Since ELL is actually optional. It is clearly described just a few >> lines below. >> > > Is it optional? This is definitely unclear. Are we looking at same README? > > commit 55e5a766f2833d6aaaf98d0f8cc250585717bc07 > Author: Nandini Rebello > Date: Wed Jan 16 12:15:31 2019 +0530 > > README says: > > Embedded Linux library > == > > In order to compile the daemon and utilities the development version > of Embedded Linux library is required to be present. The development > repositories can be found here: > > which sounds like "required" to me. Then there is: > > When using --enable-external-ell build option, it is not required that > the Embedded Linux library source code is available in the top level > directory. > > But I read it as "can be present somewhere else". > > Plus, build actually fails, with pretty cryptic error message: > > ./bootstrap-configure > ./configure > pavel@duo:~/g/ofono$ make > make --no-print-directory all-am > make[1]: *** No rule to make target 'ell/util.c', needed by > 'ell/util.lo'. Stop. > Makefile:2066: recipe for target 'all' failed > make: *** [all] Error 2 we can improve the check for the built-in copy of ELL. That is something that should be done for sure and I will look into that. However for the actual released tarballs, ELL is present. So using the an external ELL is optional. The difference is between building from the GIT tree and building from the tarballs. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH] mbim: add optional copy of TEMP_FAILURE_RETRY macro (fix musl compile)
Hi Nicolas,
> TEMP_FAILURE_RETRY is not available on musl.
>
> Signed-off-by: Nicolas Serafini
> ---
> drivers/mbimmodem/mbim.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/mbimmodem/mbim.c b/drivers/mbimmodem/mbim.c
> index 54b18acf..4b040528 100644
> --- a/drivers/mbimmodem/mbim.c
> +++ b/drivers/mbimmodem/mbim.c
> @@ -37,6 +37,16 @@
> #include "mbim-message.h"
> #include "mbim-private.h"
>
> +/* taken from glibc unistd.h for musl support */
> +#ifndef TEMP_FAILURE_RETRY
> +#define TEMP_FAILURE_RETRY(expression) \
> + (__extension__ \
> +({ long int __result; \
> + do __result = (long int) (expression); \
> + while (__result == -1L && errno == EINTR); \
> + __result; }))
> +#endif
> +
or you use the notation that is used in src/storage.h and maybe even move it to
a more common place. The TFR macro is used in various places.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono
Re: Trouble compiling ... Re: ofono fails to compile on gcc-6.3
Hi Denis, > CC drivers/rilmodem/network-registration.o > drivers/rilmodem/network-registration.c:40:32: error: unknown option > after ‘#pragma GCC diagnostic’ kind [-Werror=pragmas] >#pragma GCC diagnostic ignored "-Wrestrict" > cc1: error: unrecognized command line option ‘-Wno-format-truncation’ > [-Werror] > cc1: all warnings being treated as errors > > I commented out #pragma, and this allows compilation to > continue... until I hit same issue in > drivers/rilmodem/call-forwarding.c:41:32: >>> >>> Yeah, I'm not sure what to do with these. Marcel put in the #pragmas to >>> silence some warnings on GCC 8.x, but it seems GCC 6 doesn't know about >>> these #pragma directives and complains. >> /* Test for GCC > 3.2.0 */ >> #if __GNUC__ > 3 || \ >> (__GNUC__ == 3 && (__GNUC_MINOR__ > 2 || \ >>(__GNUC_MINOR__ == 2 && \ >> __GNUC_PATCHLEVEL__ >> > 0)) >> #pragma... >> #endif >> ? > > Marcel is taking care of the build system so I let him comment, but that > would be fine with me. Maybe you want to send something like this to the list > as a patch? > > Of course the real fix is to rewrite rilmodem properly so none of this is > needed... > >> Would it be possible to get reasonable error message when ell is not >> found? >> "ell not found, you need to install it, please see README file"? It >> took me a while to figure out... > > Sounds like a good idea. Marcel? we can try to improve the ELL detection especially since we now have internal vs external. However I need to look into this for all projects and not just oFono. So we might start with iwd here. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH] document getting of ell, point people at useful docs
Hi Pavel, > Currently there's README and HACKING... each describing part of what > needs to be done to build ofono. > > Provide some cross-links and fix build example. > > diff --git a/HACKING b/HACKING > index 15ea291..b1bd599 100644 > --- a/HACKING > +++ b/HACKING > @@ -45,7 +45,8 @@ bootstrap-configure since it could export development > specific settings. > So the normal steps to checkout, build and install such a repository is > like this: > > - Checkout repository > + Checkout repositories > +# git clone git://git.kernel.org/pub/scm/libs/ell/ell.git > # git clone git://git.kernel.org/pub/scm/network/ofono/ofono.git > # cd ofono > so coming to think about it, I would actually make this a pre-step. Checkout Embedded Linux Library # git clone .. Also this documentation is primarily for maintainers and the few that deal with making the tarballs. Which is pretty much just me. > diff --git a/README b/README > index 45bb2e9..04a50a5 100644 > --- a/README > +++ b/README > @@ -16,11 +16,14 @@ To configure run: > ./configure --prefix=/usr --mandir=/usr/share/man \ > --sysconfdir=/etc --localstatedir=/var > > -Configure automatically searches for all required components and packages. > +Configure automatically searches for all required components and > +packages, except ell, mentioned below. > Nope. Since ELL is actually optional. It is clearly described just a few lines below. > To compile and install run: > make && make install > > +See "HACKING" file for details and step-by-step examples. > + And that is wrong as well. HACKING is for the developers and people just installing oFono should not look at that. In addition HACKING is not included in the tarballs (while README is) and so you not reference a file from README that is not in the tarballs. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: sd-bus
Hi Giacinto, > > would it be interesting to port ofono from GDBus, which comes with the > > huge GLib dependency, to sd-bus? > > It is a lot of work, but the main question is whether we can assume > > that systemd is deployed enough to allow the switch. > > actually ELL provides its own D-Bus support. So the switch should towards ELL > D-Bus and not sd-bus. oFono already links against ELL and at some point that > should be its only dependency. We also want to remove the GLib dependency as > well. Newer projects like iwd for example solely use ELL. > > thank you for this information. I didn't notice the dbus in ELL, at least not > in the part currently in ofono. > Is this another implementation than glib or a part of it, or is it a wrapper > for the libdbus? it is a from-scratch and clean implementation of the D-Bus protocol. You currently don’t see it since the build magic in oFono only copies ELL pieces that it currently needs. However it is simple change in Makefile.am to also include ELL D-Bus. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: sd-bus
Hi Giacinto, > would it be interesting to port ofono from GDBus, which comes with the > huge GLib dependency, to sd-bus? > It is a lot of work, but the main question is whether we can assume > that systemd is deployed enough to allow the switch. actually ELL provides its own D-Bus support. So the switch should towards ELL D-Bus and not sd-bus. oFono already links against ELL and at some point that should be its only dependency. We also want to remove the GLib dependency as well. Newer projects like iwd for example solely use ELL. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH 1/3] sim800: add sim800 support.
Hi Clement,
>>> ---
>>> plugins/sim900.c | 91
>>> ++--
>>> 1 file changed, 82 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/plugins/sim900.c b/plugins/sim900.c
>>> index a7728cd..bf7b9ec 100644
>>> --- a/plugins/sim900.c
>>> +++ b/plugins/sim900.c
>>> @@ -24,6 +24,7 @@
>>> #endif
>>>
>>> #include
>>> +#include
>>> #include
>>> #include
>>> #include
>>> @@ -60,13 +61,77 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net:
>>> ", "SMS: ",
>>>
>>> static const char *none_prefix[] = { NULL };
>>>
>>> +typedef enum type {
>>> + SIM800,
>>> + SIM900,
>>> + SIM_UNKNOWN,
>>
>> maybe SIMCOM_UNKNOWN ?
>
> Yes, I do agree, updating patchset.
and no typedef please.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono
Re: [RFC PATCH] new gemalto plugin
Hi Giacinto, So you might need to expand on this some more. What is QMI+AT or MBIM+AT actually doing? Is there a single AT port? Multiple? What is the AT port being used for, just vendor specific APIs or something more? >>> >>> MBIM and QMI are actually used only for the gprs-context atom. >>> The modems can work with the mbim or gobi plugin if they support the >>> respective protocols (I have tested them with changes in udevng), but >>> then we only get the bare minimum: no configuration options, hardware >>> monitor, low-power mode, gnss and its options, and the rest. >>> So, given that we need the AT interface anyway, everything is done >>> through it. It is also easier to debug than the big binary blog >>> (admittedly, wireshark helps a lot for mbim, but not sure for qmi). >>> >> >> You really need to decide what you want to do here. If your modem is >> MBIM, then MBIM driver needs to be used. I really don't want to get >> into situations where we build some Frankenstein driver with >> gprs-context being driven via MBIM and netreg being driven via AT commands. >> >> I can understand if you have MBIM + a GPS/GNSS port and want to expose >> location-reporting on that. Ok, fine, I get that. I can also >> understand if you want to use the AT command port for some vendor >> specific extensions not available via MBIM. But I would need serious >> convincing of anything beyond this. >> >>> MBIM support works a bit differently depending on the chipset >>> manufacturer. The drivers/mbim/gprs-context works out of the box for >>> some models, while for others it is necessary to create the PDP >>> context with AT+CGDCONT. But this is dealt with a dedicated atom, only >>> the atom selection is visible in the plugin. >> >> So explain that one to me? You send all context details in >> MBIM_CID_CONNECT. So why would a CGDCONT be needed? >> >>> >>> enough for the detour. Back to the plug-in initialization: the >>> different models have zillion of differences. Whenever possible, I >>> prefer to ask the modem, otherwise it is if/switch depending on the >>> model. >>> Example of the first case: all Gemalto modems support the AT^SAIC >>> command if there is voice support (I checked down to the MC55 that >>> were like rel.98 or earlier). >> >> You do realize that all this probing can be done in the individual atom >> driver, right? Even as far as the driver being able to call >> ofono_foo_remove if support is lacking. >> >>> All switches are independent from each other, working on a simple >>> principle, like for voice: ascertain if the feature is present and >>> with which options, then select the right atom and atom options. >>> There are quite a few, but maintainable thanks to that. Unfortunately >>> not all features can be probed at startup because some require to be >>> online, so the checks are split in 3 parts: pre-defined with PID, >>> tested during the enable phase (could be shifted to pre-sim to have a >>> faster enable), and in post-online. >> >> We've been through this, no AT commands should be executed in pre_sim, >> post_sim, post_online hooks... >> >>> And this is the part to maintain. If I split the plugins in 3 >>> independent ones, I need to duplicate and maintain this. >> >> No, you really don't. It might take a while for you to be convinced though. >> >>> mbim/qmi/plainAt will disappear. And all the vendor-specific atoms >>> will have to be duplicated too. >> >> Again, they really don't need to be duplicated... >> Since it sounds like this is a very esoteric use case, you might want to schedule this last. Right now it just distracts from the core discussion. >>> >>> ok for having it last, but it is not so esoteric. >>> Take for example the audio settings (which most likely exist for all >>> other vendors): you can select the type of interface >>> (analog/digital/usb), the port to use in case there is more than one, >>> suboptions like I2C or PCM, master/slave, and quite some other. >>> They could be set in a quite large vendor-specific interface (I saw >>> some tried it in an atom), but the configuration is fixed for the end >>> application, and - as you explained - will raise more problems than it >>> solves exposing these settings through dbus. >>> So the best is to run a couple of commands to set the interface >>> properly for the product and that's it. >> >> I understand. But this is also why we have audio-settings atom. So all >> such details can be easily put in there and its relevant driver. If you >> want to have some simple configuration file that is read by your driver >> where the user can select between some common configurations that is >> fine as well. >> >> One can also add additional drivers / plugins out-of-tree that would be >> dynamically loaded and do whatever they desire. But reading a file and >> blindly executing AT commands in a daemon that is running as root is >> just asking for trouble. I don't really care what you do
Re: [RFC PATCH] new gemalto plugin
Hi Denis, >>> So you might need to expand on this some more. What is QMI+AT or >>> MBIM+AT actually doing? Is there a single AT port? Multiple? What is >>> the AT port being used for, just vendor specific APIs or something more? >> MBIM and QMI are actually used only for the gprs-context atom. >> The modems can work with the mbim or gobi plugin if they support the >> respective protocols (I have tested them with changes in udevng), but >> then we only get the bare minimum: no configuration options, hardware >> monitor, low-power mode, gnss and its options, and the rest. >> So, given that we need the AT interface anyway, everything is done >> through it. It is also easier to debug than the big binary blog >> (admittedly, wireshark helps a lot for mbim, but not sure for qmi). > > You really need to decide what you want to do here. If your modem is MBIM, > then MBIM driver needs to be used. I really don't want to get into > situations where we build some Frankenstein driver with gprs-context being > driven via MBIM and netreg being driven via AT commands. > > I can understand if you have MBIM + a GPS/GNSS port and want to expose > location-reporting on that. Ok, fine, I get that. I can also understand if > you want to use the AT command port for some vendor specific extensions not > available via MBIM. But I would need serious convincing of anything beyond > this. I think that MBIM (and even QMI) have AT passthrough options. So by all means, the main transport suppose to be MBIM here. I always prefer if only one hardware port is opened and there is no need to open more. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH 1/1] Remove superfluous definitions of _GNU_SOURCE
Hi Jonas, >>> There are a large number of files in the tree that define _GNU_SOURCE >>> despite not actually using features hidden behind this flag. This patch >>> removes all these definitions in one fell swoop... >>> --- >>> >>> I realize that it's not quite kosher in ofono to send pathes that touch >>> more than directory. Breaking this up into separate patches doesn't >>> really make sense though, given its nature. If the list insists, >>> though, I can break this up into a series of ~50 patches... >>> >>> /Jonas >> Do all files and the project compile? > > Yes. At least with the configuration I've got, which is mostly the default. > > There are ~5 files where _GNU_SOURCE features are actually used. Those, of > course, retain the definition. I found musl most strict in this regards while glibc is rather relaxed. If this compiles with musl cleanly, I am fine. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: commit d37c22be20c83cf370638a9bad243bc5219c5509
Hi Giacinto, > > was it necessary to remove entirely the NEED_THREADS flag? > > > > I was going to use it in the Gemalto driver, to speed up the device > > recognition, because for some modules it is hit and miss. > > But only as an optional feature, because in some systems it might be > > missing. > > > > Also, for some high-speed modems which take long to boot, there is a > > potential critical race in the recognition that the use threads can solve > > easily. The alternative is to increase the internal timers a lot. > > > > I likely need to add a new flag for it. > > using threads is not the solution. This can be all done properly with single > thread event loop driven methods. > > for sure, and in fact as of now the support is optional. > But I already need to ask for an extension of the Powered timout from 20 to > 60 seconds because several LTE chipsets take at least 30-40 seconds to boot > once they have already enumerated on USB. It would be good to shorten this > time. > > Besides, several modules use the option linux driver, which blocks in case > the port doesn't answer. > This single call: > g_at_chat_unref(port); > blocks either 30s or 1 minute, depending on the system. I might step into it > twice during my hardware initialization for several 3G models. > It helps to run this line on a separate thread unblocking the rest. no it does not. Nothing in GAtChat is thread safe. So running this in a separate thread just means you are having tons of race conditions on your hand. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: commit d37c22be20c83cf370638a9bad243bc5219c5509
Hi Giacinto, > was it necessary to remove entirely the NEED_THREADS flag? > > I was going to use it in the Gemalto driver, to speed up the device > recognition, because for some modules it is hit and miss. > But only as an optional feature, because in some systems it might be missing. > > Also, for some high-speed modems which take long to boot, there is a > potential critical race in the recognition that the use threads can solve > easily. The alternative is to increase the internal timers a lot. > > I likely need to add a new flag for it. using threads is not the solution. This can be all done properly with single thread event loop driven methods. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: Voice calls over qmi was Re: Incoming sms problem on Motorola Droid 4
Hi Joey, > nice to hear, somebody else is using it. > > The voicecall driver needs to be refactored as Denis already pointed > out on the mailinglist before it can go upstream. > > The original idea of the voice_generated.c/h files was to create a > prototype how qmi generated files should look like and then write a > code generator for it, as the other qmi related projects already do. I think that code generation is a bad idea. And other projects are drinking some cool aid. So I really prefer not to have generated code for QMI support upstream. It is also a total waste of time. Using QMI is actually really simple. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: Incoming sms problem on Motorola Droid 4
Hi Pavel, If I attempt to do AT+CNMA=1, AT+CNMA=0 or just plain At=CNMA, it does not like that, either, but with +CMS ERROR: 500. >>> >>> CNMA=1 is required to work in PDU mode according to 27.005 btw. How >>> about: >>> >>> window.open(); >>> window.throw(modem); >> >> Heh :-). Well, there are two phones with reasonable Linux support: >> N900 and Droid 4. N900 can't do voice calls of reasonable >> quality. That leaves Droid 4... so I'd really like it to work. > > Is this actually an AT command modem or one of those modems where AT > commands were only bolted on for carrier certification? Does it support > QMI > or something maybe? Ok, so ... this is complex. And maybe this is one of "those" modems. There's USB with AT commands, USB with QMI protocol, then serial with mux and U1234AT+XYZ commands. It seems Android uses serial, but that is being worked on, so I'm using AT commands for now. >>> >>> Wow, okay. You might want to try using the qmi driver family instead. >> >> I would switch to QMI since then you can also ignore this weird multiplexer. > > More explanation needed it seems: > > I can already do USB with AT commands, no multiplexer needed. We'd > actually prefer to go over serial (with multiplexer), because USB > needs to be powered down in idle, and you still need communication > ovre serial to know that wakeup of USB is needed. > > Yes, it is complex :-(. I missed the part that the weird serial multiplexer is wired independently. Then do everything via that serial multiplexer and never look back. However if that is as broken, then you will be really out of luck. Making this work reliable will be really hard. While we have seen QMI protocol issues as well, but they have been less bad than the bolted on AT commands. Manufactures always get AT commands wrong since they let humans test it and not a real telephony stack like oFono. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: Incoming sms problem on Motorola Droid 4
Hi Pavel, >> If I attempt to do AT+CNMA=1, AT+CNMA=0 or just plain At=CNMA, it does >> not like that, either, but with +CMS ERROR: 500. > > CNMA=1 is required to work in PDU mode according to 27.005 btw. How > about: > > window.open(); > window.throw(modem); Heh :-). Well, there are two phones with reasonable Linux support: N900 and Droid 4. N900 can't do voice calls of reasonable quality. That leaves Droid 4... so I'd really like it to work. >>> >>> Is this actually an AT command modem or one of those modems where AT >>> commands were only bolted on for carrier certification? Does it support QMI >>> or something maybe? >> Ok, so ... this is complex. And maybe this is one of "those" modems. >> There's USB with AT commands, USB with QMI protocol, then serial with >> mux and U1234AT+XYZ commands. >> It seems Android uses serial, but that is being worked on, so I'm >> using AT commands for now. > > Wow, okay. You might want to try using the qmi driver family instead. I would switch to QMI since then you can also ignore this weird multiplexer. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: Incoming sms problem on Motorola Droid 4
Hi Pavel, >> I have problems with incoming SMS. ofono tries to use +CNMI=1,2,2,1,0 >>> AT+CNMI=? >> < +CNMI: (0,1,2),(0,1,2,3),(0,2),(0,1,2),(0,1) >> < OK >> ofonod[3070]: drivers/atmodem/sms.c:build_cnmi_string() >> ofonod[3070]: drivers/atmodem/sms.c:construct_ack_pdu() >>> AT+CNMI=1,2,2,1,0 >> < OK >> ofonod[3070]: src/network.c:__ofono_netreg_add_status_watch() 0x5bbbf0 >> ... unfortunately, with that configuration no messages are comming to >> ofono and the other phone sees them as "delievery failed". > > So you're saying the modem firmware is lying about supporting all these modes > :) > >> I had some luck with unicsy_demo using AT+CNMI=1,2 with text mode (not >> PDU) messages. That works well enough for me. > > oFono doesn't support text mode… and it is impossible to support in a sane and complete manner. >> Unfortunately, if I force ofono to pass "AT+CNMI=1,2", it does not >> work well, either. > > You could try using value of '1' for the parameter and see if the modem > will route messages to memory first and send a +CMTI indication.. I remember vaguely that I dealt with a modem that had some issue with the indications. Maybe we already have a quirk for that in one of the drivers. So look for SIMCOM or CINTERION vendor quirks. There are bunch of comments in drivers/atmodem/sms.c for this. You need to figure out on what level this modem is broken. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH 0/2] gobi: support SMD devices (smartphone SoC)
Hi Joey, >> I would still fix this upstream and only workaround devices blacklisted with >> a quirk. > > I don't see how this would be possible. The issue with poll() lies in the > kernel, not the device. I guess a patched kernel could somehow indicate that > it supports poll() for writing, like an attr on the device node. > > OK, there's probably a lot I don't know about rpmsg, and I don't currently > have a device running this setup. it is a kernel bug for sure and should be fixed there. Not supporting POLLOUT properly is just bad. >> This reminds me also why RPMSG is not actually exported as a socket in the >> first. Creating a device node first and then using it later is always prone >> to race conditions. An alternative is to open the control character device >> and turn it into a proper pipe for QMI. I would like to see some proper >> fixes in the RPMSG subsystem to allow for proper handling by a telephony >> daemon. > I don't understand; what is the race condition? oFono plus another client > trying to use the same device? > > How would the control device become a QMI pipe? As it stands today, I think > all you can do with it is open and do that ioctl > (https://github.com/torvalds/linux/blob/60cc43fc888428bb2f18f08997432d426a243338/drivers/rpmsg/rpmsg_char.c#L451). > How is the rpmsg endpoint char device not a proper pipe for QMI, compared to > what you're envisioning? > > It sounds like the userspace rpmsg system is not mature enough to do this the > way you want. Will we have to wait until then to merge this? Sadly, I have seen this before where it is not fully thought through. It could be really great and race free, but seems right now it is only half done. So I would start improving RPMSG subsystem. We can look into working around older kernels (with tons of comments on why), but first I would like to see if we can do this properly. So that we don’t have to workaround our own workaround once the kernel behaves better. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH 0/2] gobi: support SMD devices (smartphone SoC)
Hi Joey, > These add support for Qualcomm-based smartphone modems. > > The first is fairly straightforward: searching for "rpmsg"-subsystem devices > and supporting them in gobi as a serial device. A friend has tested it on the > mainline kernel, with the help of something like `rpmsgexport > /dev/rpmsg_ctrl0 DATA5_CNTL` (https://github.com/andersson/rpmsgexport) and > then giving the created rpmsg node OFONO_DRIVER=gobi property. I would actually prefer that the functionality of RPMSG_CREATE_EPT_IOCTL gets done inside oFono’s plugin. Also lifetime rules of that created node are wonky. I would expect that oFono quits, they are returned back to RPMSG subsystem. > The second is uglier, and maybe controversial, since it's for Qualcomm's > forked kernel. The smdpkt device doesn't support polling for write, so oFono > would wait forever on the poll. This would probably be a 5-line fix in the > kernel, but there are thousands of kernel forks, so I thought it might be > better to workaround in oFono. I've tested it on Samsung S4 Mini LTE, running > a kernel based on the MSM fork. I would still fix this upstream and only workaround devices blacklisted with a quirk. This reminds me also why RPMSG is not actually exported as a socket in the first. Creating a device node first and then using it later is always prone to race conditions. An alternative is to open the control character device and turn it into a proper pipe for QMI. I would like to see some proper fixes in the RPMSG subsystem to allow for proper handling by a telephony daemon. On a side note, we had all these race conditions and issues with GSM 07.07 multiplexer code before. Having to wait for a device node to be create is crap. One reason why oFono comes with a builtin GSM 07.07 implementation. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH 1/1] doc: Fix documentation for HFP memory dialling
Hi Philippe, > Signed-off-by: Philippe De Swert No signed-off-by please. > --- > doc/voicecallmanager-api.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/doc/voicecallmanager-api.txt b/doc/voicecallmanager-api.txt > index 5c4ce11b..96492ced 100644 > --- a/doc/voicecallmanager-api.txt > +++ b/doc/voicecallmanager-api.txt > @@ -69,10 +69,10 @@ Methods dict GetProperties() >[service].Error.NotImplemented >[service].Error.Failed > > - object DialMemory(string memory position, string hide_callerid) > + object DialMemory(unsigned int memory position) And there is no unsigned int in D-Bus. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: IPV6 question
Hi Harald, > This might be off topic, but AFAIR there are PPP type PDP contexts where not > raw IP, but actual PPP frames are transmitted over GPRS inside SNDCP/LLC. Not > sure if any operators use that, but its perfectly valid. > > In this case PPP doesn't terminate in the modem, so "hardware" support should > be irrelevant. I have heard about these PPP type PDT contexts, but I have not seen any of these deployed. They might do exists, but they seem like such an odd duck that it makes no sense to worry too much about it. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH 2/2] don't redefine GNU_SOURCE
Hi Petr, > Signed-off-by: Petr Vorel > --- > drivers/atmodem/call-barring.c | 2 ++ > drivers/atmodem/call-forwarding.c | 2 ++ > drivers/atmodem/call-meter.c| 2 ++ > drivers/atmodem/call-settings.c | 2 ++ > drivers/atmodem/call-volume.c | 2 ++ > drivers/atmodem/cbs.c | 2 ++ > drivers/atmodem/gnss.c | 2 ++ > drivers/atmodem/gprs-context.c | 2 ++ > drivers/atmodem/gprs.c | 2 ++ > drivers/atmodem/network-registration.c | 2 ++ > drivers/atmodem/phonebook.c | 2 ++ > drivers/atmodem/sim-auth.c | 2 ++ > drivers/atmodem/sim.c | 2 ++ > drivers/atmodem/sms.c | 2 ++ > drivers/atmodem/stk.c | 2 ++ > drivers/atmodem/ussd.c | 2 ++ > drivers/atmodem/voicecall.c | 2 ++ > drivers/calypsomodem/stk.c | 2 ++ > drivers/calypsomodem/voicecall.c| 2 ++ > drivers/cdmamodem/connman.c | 2 ++ > drivers/cdmamodem/voicecall.c | 2 ++ > drivers/hfpmodem/call-volume.c | 2 ++ > drivers/hfpmodem/handsfree.c| 2 ++ > drivers/hfpmodem/network-registration.c | 2 ++ > drivers/hfpmodem/siri.c | 2 ++ > drivers/hfpmodem/slc.c | 2 ++ > drivers/hfpmodem/voicecall.c| 2 ++ > drivers/hsomodem/gprs-context.c | 2 ++ > drivers/hsomodem/radio-settings.c | 2 ++ > drivers/huaweimodem/audio-settings.c| 2 ++ > drivers/huaweimodem/cdma-netreg.c | 2 ++ > drivers/huaweimodem/gprs-context.c | 2 ++ > drivers/huaweimodem/radio-settings.c| 2 ++ > drivers/huaweimodem/voicecall.c | 2 ++ > drivers/iceramodem/gprs-context.c | 2 ++ > drivers/iceramodem/radio-settings.c | 2 ++ > drivers/ifxmodem/audio-settings.c | 2 ++ > drivers/ifxmodem/ctm.c | 2 ++ > drivers/ifxmodem/gprs-context.c | 2 ++ > drivers/ifxmodem/radio-settings.c | 2 ++ > drivers/ifxmodem/stk.c | 2 ++ > drivers/ifxmodem/voicecall.c| 2 ++ > drivers/isimodem/audio-settings.c | 2 ++ > drivers/isimodem/call-barring.c | 2 ++ > drivers/isimodem/call-forwarding.c | 2 ++ > drivers/isimodem/call-meter.c | 2 ++ > drivers/isimodem/call-settings.c| 2 ++ > drivers/isimodem/cbs.c | 2 ++ > drivers/isimodem/debug.c| 2 ++ > drivers/isimodem/devinfo.c | 2 ++ > drivers/isimodem/gprs-context.c | 2 ++ > drivers/isimodem/gprs.c | 2 ++ > drivers/isimodem/network-registration.c | 2 ++ > drivers/isimodem/radio-settings.c | 2 ++ > drivers/isimodem/sim.c | 2 ++ > drivers/isimodem/sms.c | 2 ++ > drivers/isimodem/uicc-util.c| 2 ++ > drivers/isimodem/uicc.c | 2 ++ > drivers/isimodem/ussd.c | 2 ++ > drivers/isimodem/voicecall.c| 2 ++ > drivers/mbmmodem/gprs-context.c | 2 ++ > drivers/mbmmodem/location-reporting.c | 2 ++ > drivers/mbmmodem/stk.c | 2 ++ > drivers/nwmodem/radio-settings.c| 2 ++ > drivers/qmimodem/location-reporting.c | 2 ++ > drivers/qmimodem/qmi.c | 2 ++ > drivers/rilmodem/call-forwarding.c | 2 ++ > drivers/rilmodem/call-settings.c| 2 ++ > drivers/rilmodem/call-volume.c | 2 ++ > drivers/rilmodem/devinfo.c | 2 ++ > drivers/rilmodem/gprs-context.c | 2 ++ > drivers/rilmodem/gprs.c | 2 ++ > drivers/rilmodem/network-registration.c | 2 ++ > drivers/rilmodem/phonebook.c| 2 ++ > drivers/rilmodem/radio-settings.c | 2 ++ > drivers/rilmodem/sim.c | 2 ++ > drivers/rilmodem/sms.c | 2 ++ > drivers/rilmodem/ussd.c | 2 ++ > drivers/rilmodem/voicecall.c| 2 ++ > drivers/stemodem/gprs-context.c | 2 ++ > drivers/stemodem/radio-settings.c | 2 ++ > drivers/stemodem/voicecall.c| 2 ++ > drivers/swmodem/gprs-context.c | 2 ++ > drivers/telitmodem/location-reporting.c | 2 ++ > drivers/ztemodem/radio-settings.c | 2 ++ > gisi/client.c | 2 ++ > gisi/modem.c| 2 ++ > gril/parcel.c | 2 ++ > src/call-volume.c | 2 ++ > src/cdma-smsutil.c | 2 ++ > src/common.c| 2 ++ > src/gnss.c | 2 ++ > src/gnssagent.c | 2 ++ > src/handsfree.c | 2 ++ > src/idmap.c | 2 ++ > src/log.c | 2 ++ > src/phonebook.c | 2 ++ > src/sim-auth.c | 2 ++ > src/sim.c | 2 ++ > src/simfs.c | 2 ++ > src/siri.c | 2 ++ > src
Re: Problems provisioning APN from SIMs
Hi Alex, Ordering should have nothing to do with it. >>> >>> Yes, the ordering is relevant. We (like other ofono users I suspect) >>> have to allow multiple APNs or the automatic provisioning process fails. >>> >>> Then, the first context found in serviceproviders.xml is what is used by >>> default for the connection. >>> >>> An example of the problem is that if you use a major telco's SIM card in >>> the UK - Vodafone, ofono will then default to using an ASDA mobile >>> context because of the ordering, and this will fail. >>> >>> My feeling is that a larger provider like Vodafone or O2 should be the >>> default, not ASDA mobile or GiffGaff, and this should thus come first >>> (understanding that the Ofono project does not control this document) >> >> It has been years since I wrote the provisioning plugin, but the >> intent was to fail if looking up MCC/MNC combo resulted in multiple >> matches. So this may be a bug, or you might be using some custom >> behavior. But in the end, ordering of the entries should not affect >> the provisioning logic. >> >>> Allowing Duplicates - Not by default no, but you have a boolean >>> parameter in there and logic to allow for duplicate contexts, which we >>> have to enable (as do others I think from my Googling on this) or the >>> provisioning support is unusable with the upstream serviceproviders.xml >>> as far as I can see. >> >> Then that's the problem. The intent was never to allow duplicates. >> That boolean was added for tools/lookup-apn only. >> >>> >>> I'm not entirely sure how the RilModem fork relates to Ofono but you can >>> see they had the same problem >>> >>> /* >>> >>>* TODO: review with upstream. Default behavior was to >>> >>>* disallow duplicate APN entries, which unfortunately exist >>> >>>* in the mobile-broadband-provider-info db. >>> >>>*/ >>> >>> >>> ref: >>> https://github.com/rilmodem/ofono/blob/master/plugins/provision.c#L55 >>> >>> SPN - Thanks. This seems promising. I will investigate the SPN values >>> further. >> >> The real fix is to fix mobile-broadband-provider-info. > > Yes I would agree with that. > > As I come to investigate this, I find I am concerned about using the > Service Provider Name as I can't see any registry for those names, it's > free text for display purposes, so I assume it is at least possible it > might change without warning, > whereas there does seem to be a registry for MCC/MNC (e.g. > http://www.mcc-mnc.com/) > > I am thinking it may be preferable to use the registered IIN number from > the ICCID - http://www.controlf.net/iccid/ > > This seems a more controlled way of providing the uniqueness needed to > me and presumably it's easy enough to read the ICCID out, if it's not > already being read out. and I stand corrected here. The TS.25 (formerly known as SE13) is just for collection of operator and networks if the need arises by the equipment to map them. The actual MCC and MNC assignments are ITU T E.212 and the (U)SIM Header of the ICCID is ITU T E.118 document. And as a side note, the (U)SIM Header is between 6 and 7 digits. The MNC is between 2 and 3 digits. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: Problems provisioning APN from SIMs
Hi Alex, Ordering should have nothing to do with it. >>> >>> Yes, the ordering is relevant. We (like other ofono users I suspect) >>> have to allow multiple APNs or the automatic provisioning process fails. >>> >>> Then, the first context found in serviceproviders.xml is what is used by >>> default for the connection. >>> >>> An example of the problem is that if you use a major telco's SIM card in >>> the UK - Vodafone, ofono will then default to using an ASDA mobile >>> context because of the ordering, and this will fail. >>> >>> My feeling is that a larger provider like Vodafone or O2 should be the >>> default, not ASDA mobile or GiffGaff, and this should thus come first >>> (understanding that the Ofono project does not control this document) >> >> It has been years since I wrote the provisioning plugin, but the >> intent was to fail if looking up MCC/MNC combo resulted in multiple >> matches. So this may be a bug, or you might be using some custom >> behavior. But in the end, ordering of the entries should not affect >> the provisioning logic. >> >>> Allowing Duplicates - Not by default no, but you have a boolean >>> parameter in there and logic to allow for duplicate contexts, which we >>> have to enable (as do others I think from my Googling on this) or the >>> provisioning support is unusable with the upstream serviceproviders.xml >>> as far as I can see. >> >> Then that's the problem. The intent was never to allow duplicates. >> That boolean was added for tools/lookup-apn only. >> >>> >>> I'm not entirely sure how the RilModem fork relates to Ofono but you can >>> see they had the same problem >>> >>> /* >>> >>>* TODO: review with upstream. Default behavior was to >>> >>>* disallow duplicate APN entries, which unfortunately exist >>> >>>* in the mobile-broadband-provider-info db. >>> >>>*/ >>> >>> >>> ref: >>> https://github.com/rilmodem/ofono/blob/master/plugins/provision.c#L55 >>> >>> SPN - Thanks. This seems promising. I will investigate the SPN values >>> further. >> >> The real fix is to fix mobile-broadband-provider-info. > > Yes I would agree with that. > > As I come to investigate this, I find I am concerned about using the > Service Provider Name as I can't see any registry for those names, it's > free text for display purposes, so I assume it is at least possible it > might change without warning, > whereas there does seem to be a registry for MCC/MNC (e.g. > http://www.mcc-mnc.com/) actually the TS.25 database from GSMA is the authority. Seems to have over 1800 entries at the moment, but a lot of operators are listed more than once. I think it is a combination of band + MCC + MNC. The SPN is indeed free form and can change at any time (even during a phone call). They are however a really good indication when you want to differentiate MNVO. But it means you have to update your database. However that needs to happen anyway since even MCC and MNC change all the time. I think that TS.25 updates twice per month. So some operators are actually assigned parts of the IMSI to their MNVO. I have seen operator documents clearly identifying which part of their ISMI pool is for MNVO a, b, c etc. However that is the exception and not the rule. However one big operator could be easily and clearly identified if you get their documents. > I am thinking it may be preferable to use the registered IIN number from > the ICCID - http://www.controlf.net/iccid/ > > This seems a more controlled way of providing the uniqueness needed to > me and presumably it's easy enough to read the ICCID out, if it's not > already being read out. As you saw by yourself, that is not going to help. The ICCID is pretty much useless in this regard. It is listed in the TS.25 database, but I have no idea how accurate that is. Some operators only list the first 2 digits and some have no entry at all. Keep in mind that TS.25 does not list MNVO and with that I am thinking that ICCID prefix is free to assign by the operator. Same as the decision to use parts of the IMSI to identify a MVNO. Some of them might really do not care at all and just map this internally without visibility to the user equipment. The reason why you can write your own provision plugin and create your own database is exactly this. oFono does not want to be in this business. That is up to the end product to put a proper database in place or write a smarter plugin that can use extra information from the device or SIM card. It is plugin based for that exact reason. And I am surely providing something better than mobile-broadband-provider-info is easy and worth while doing. It is just not an oFono objective. One thing we spent a lot of time on is to get the MNC length correct. Since it can be 2 or 3 digits, we do make sure that whatever modem quirk it takes, we read the right value from the SIM card. Without this you would be in an even worse position. Regards Marcel
Re: [PATCH] gprs: Setup route for mmsc if there's no mms proxy
Hi Slava,
> And don't setup mms route at all if mmsc/proxy host name is not in
> dotted IPv4 format.
> ---
> src/gprs.c | 25 -
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/src/gprs.c b/src/gprs.c
> index 05ab499..9c643b9 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -585,13 +585,16 @@ static void pri_context_signal_settings(struct
> pri_context *ctx,
> context_settings_append_ipv6);
> }
>
> -static void pri_parse_proxy(struct pri_context *ctx, const char *proxy)
> +static gboolean pri_parse_proxy(struct pri_context *ctx, const char *proxy)
> {
> char *scheme, *host, *port, *path;
>
> + if (proxy[0] == 0)
> + return FALSE;
> +
> scheme = g_strdup(proxy);
> if (scheme == NULL)
> - return;
> + return FALSE;
>
> host = strstr(scheme, "://");
> if (host != NULL) {
> @@ -604,7 +607,7 @@ static void pri_parse_proxy(struct pri_context *ctx,
> const char *proxy)
> ctx->proxy_port = 80;
> else {
> g_free(scheme);
> - return;
> + return FALSE;
> }
> } else {
> host = scheme;
> @@ -626,10 +629,16 @@ static void pri_parse_proxy(struct pri_context *ctx,
> const char *proxy)
> }
> }
>
> + if (host[0] == 0) {
> + g_free(scheme);
> + return FALSE;
> + }
> +
> g_free(ctx->proxy_host);
> ctx->proxy_host = g_strdup(host);
>
> g_free(scheme);
> + return TRUE;
> }
There is a single call instance for pri_parse_proxy. Why are we not checking
right there if the ctx->message_proxy is something that is worth while parsing.
> static void pri_ifupdown(const char *interface, ofono_bool_t active)
> @@ -715,11 +724,16 @@ static void pri_setproxy(const char *interface, const
> char *proxy)
> {
> struct rtentry rt;
> struct sockaddr_in addr;
> + in_addr_t proxy_addr;
> int sk;
>
> if (interface == NULL)
> return;
>
> + proxy_addr = inet_addr(proxy);
> + if (proxy_addr == INADDR_NONE)
> + return;
> +
> sk = socket(PF_INET, SOCK_DGRAM, 0);
> if (sk < 0)
> return;
> @@ -730,7 +744,7 @@ static void pri_setproxy(const char *interface, const
> char *proxy)
>
> memset(&addr, 0, sizeof(addr));
> addr.sin_family = AF_INET;
> - addr.sin_addr.s_addr = inet_addr(proxy);
> + addr.sin_addr.s_addr = proxy_addr;
> memcpy(&rt.rt_dst, &addr, sizeof(rt.rt_dst));
>
> memset(&addr, 0, sizeof(addr));
This extra handling seems unneeded. We do not call pri_setproxy when
ctx->proxy_host == NULL. So what should happen is to ensure that
ctx->proxy_host is set to NULL. Then no change here is needed.
> @@ -792,7 +806,8 @@ static void pri_update_mms_context_settings(struct
> pri_context *ctx)
> if (ctx->message_proxy)
> settings->ipv4->proxy = g_strdup(ctx->message_proxy);
>
> - pri_parse_proxy(ctx, ctx->message_proxy);
> + if (!pri_parse_proxy(ctx, ctx->message_proxy))
> + pri_parse_proxy(ctx, ctx->message_center);
What is this change doing? Parsing the message center into ctx->proxy_host
makes no sense to me.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH 1/2] Allow users to specify dbus name replacement behaviour.
Hi Jussi, In testing it is sometimes useful to be able to replace the system ofono daemon instance with our own. This patch makes this possible using dbus' name replacement feature. This patch has the plumbing changes to make it possible to set the name replacement settings. The next patch allows you to set these parameters from the command line. The default behaviour does not change, i.e. the service name is not replaceable and the daemon will not try to replace an existing ofono instance. >>> >>> what is this useful for. We have been running oFono for more than 4 >>> years and BlueZ with D-Bus for over 10 years and never had the need >>> for doing this. So I do not understand why we would support this. >> >> This helps in an issue that comes up in system-wide automated testing. >> There are some tests that we want to run different ofono instances. All >> these tests need to run in the same instance and without root >> privileges. The normal approach would be to run the tests under a >> private dbus session. However this becomes problematic when the thing we >> are testing requires other services that are only provided by the real >> system bus. Permitting name transfer allows us to replace only the ofono >> instance and do so without root privileges (installing a custom dbus >> conf file that permits name replacement during testing is straightforward). > > > Indeed, and this combined with commit 5f765259 one can easily run a > series of tests (let's say dialer UI, messaging..) with different > phonesim configurations changing the number of modems and phonesim .xml > files for each modem. this argument does not really fly with me. You can just disable/enable the phonesim modem over D-Bus and it allows you to connect to a total different phonesim instance with a new phonesim.xml. Or go the route of plugins/stktest.c and tools/stktest.c that we used for unit testing SIM Toolkit. And here as well. If you are allowed to own the D-Bus well know name, you normally can kill the current process and start a new one of your choice. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH 1/2] Allow users to specify dbus name replacement behaviour.
Hi Jussi, >>> In testing it is sometimes useful to be able to replace the system >>> ofono daemon instance with our own. This patch makes this possible >>> using dbus' name replacement feature. This patch has the plumbing >>> changes to make it possible to set the name replacement settings. >>> The next patch allows you to set these parameters from the command >>> line. The default behaviour does not change, i.e. the service name >>> is not replaceable and the daemon will not try to replace an >>> existing ofono instance. >> >> what is this useful for. We have been running oFono for more than 4 >> years and BlueZ with D-Bus for over 10 years and never had the need >> for doing this. So I do not understand why we would support this. > > This helps in an issue that comes up in system-wide automated testing. > There are some tests that we want to run different ofono instances. All these > tests need to run in the same instance and without root privileges. The > normal approach would be to run the tests under a private dbus session. > However this becomes problematic when the thing we are testing requires other > services that are only provided by the real system bus. Permitting name > transfer allows us to replace only the ofono instance and do so without root > privileges (installing a custom dbus conf file that permits name replacement > during testing is straightforward). the custom D-Bus configuration needs to be installed as root. So you need the root access right there. And if oFono is not running as root, then the same oFono user can start and stop it. I do not see what problem you are trying to solve. So just kill the current ofonod process using kill(1) and start the new one. If you have access to the process that is allowed to own the name, you can also terminate the current process. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH 1/2] Allow users to specify dbus name replacement behaviour.
Hi Jussi, > In testing it is sometimes useful to be able to replace the system ofono > daemon > instance with our own. This patch makes this possible using dbus' name > replacement > feature. This patch has the plumbing changes to make it possible to set the > name replacement settings. The next patch allows you to set these parameters > from the command line. The default behaviour does not change, i.e. the service > name is not replaceable and the daemon will not try to replace an existing > ofono > instance. what is this useful for. We have been running oFono for more than 4 years and BlueZ with D-Bus for over 10 years and never had the need for doing this. So I do not understand why we would support this. > --- > dundee/main.c| 2 +- > gdbus/gdbus.h| 12 +--- > gdbus/mainloop.c | 22 ++ And gdbus/ changes must always be in a separate patch. This code is used by ConnMan, oFono, BlueZ etc. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: RFC: Ubuntu Touch, MMS, and Provisioning
Hi Tony, > What do you think about extending org.ofono.ConnectionContext API with > Add/RemoveProperty calls and PropertyAdded/Removed signals to allow > platform/application specific properties of type string? Use cases > include marking connections as preferred/default or assigning priorities > to them, tracking the origin of the context (OTA, local provision or > manually edited) and who knows what else. > There are practical considerations that make this a bad idea, e.g. security concerns or the fact that our dbus bindings do not handle dynamic properties. Putting that aside... Your suggestion would be completely against our three core design values, e.g. "Minimal and Complete"; "Consistent" and "Easy to Use". I'm open to adding additional properties the "old way" if you can argue good use cases for them. >>> >>> A couple of examples. >>> >>> Suppose, we have a UI that allows the user to switch between "Automatic" >>> and "Custom" MMS or GPRS settings. One of the ways to implement that would >>> be to create a new context marked as "manual" and allow the user to edit >>> it. The old context remains but it's marked as "automatic" or whatever. >>> Manual context has a precedence over the automatic one. Switching back to >>> automatic means destroying the "manual" context or marking it as >>> "disabled". All that stuff gets saved to the ofono SIM-specific gprs file >>> so that these context properties don't get lost after swapping SIMs. >>> >>> Suppose, we have a different connection management system, which shows the >>> entire list of available access points to the user and allows to choose >>> which one to use. The selected context needs to be marked as "default" or >>> something. Again, this context property needs to survive SIM swap. With a >>> custom context property that's pretty easy to do. >> >> I am not even sure what’s your goal here? Are just trying to plain copy the >> horrible crap that Android puts in front of their users? >> You do realize that their list of APNs just purely exists since they never >> figured out on how to do the automatic provisioning properly and user >> friendly. > > No, I didn't realize this. I've generally had positive experiences with the > Android phones ( mostly Nexus*, except for a G1 ) I've owned. it is pretty bad. I have used it in many difference countries and it gets a lot of stuff horrible confused. Fun stuff if you enter Canada and for some reason it decided to configure APNs from Italy after you get out of flight mode. It also gets re-provisioned once you switch SIM cards which makes this horrible annoying. Something that the iPhone does as well. oFono has this luckily persistent. > That said, apns-conf has a lot more content than mbpi, especially with > regards to MMS support. There are grand total of 16 MMS APNs defined in the > latest version of mbpi: > > https://git.gnome.org/browse/mobile-broadband-provider-info/ The MBPI database is pretty much a joke when it comes to MMS. It is also a joke when it comes to automated provisioning since they stuffed everything in there. So it is hard to make a choice. Then again, the Gnome guys used that for UI based configuration and for that it is not that bad. For example in one of our devices we used a combined approach. If the APN entries could be clearly identified it gets auto-provisioned by oFono’s plugin. If that failed the UI realizes this and starts a first time setup UI that lets you chose the APN based on the plan information or what not. So the database got used twice. With the information that oFono provided for the SIM cards MCC and MNC (and the MNC length determination from SIM filesystem is important here), the first time setup agent could be as smart as human possible. So it normally just showed the list of MNVOs and you had to pick one. That was it. After that it was persistently saved by oFono and the user never had to deal with these details again (unless manually triggered by user through settings). As mentioned in the other email, we realized that the MBPI is not the best database available and we made certain improvements to it already. However the content never got where it needs to be. That is why for oFono it is plugin based. You can just use your own database format and then have your plugin do the job. Personally I think some simple CSV based format would work just fine. I think that is what Nokia used in the Maemo/MeeGo devices. The real hard job is to go through all the operator information and create a good database. That is the key here. Ignoring that fact is just silly. >> I have to second Denis here that time is better spent in creating a proper >> provision database instead of just copying what AOSP provides. > > Sure, but how long has mbpi been around for, and why isn't it in better > shape? Also, as explained earlier Android supports
Re: RFC: Ubuntu Touch, MMS, and Provisioning
Hi Tony, >>> We've been working on MMS support for Ubuntu Touch recently and have run >>> into a couple of stumbling blocks, so I have a few questions about the >>> current MMS logic in oFono ( we're still 1.12 based ), and in particular >>> the provisioning/management of gprs-contexts. >>> >>> As part of this work, we're planning to switch from using the builtin gprs >>> 'provision' plugin, to the new android-provision/apndb plugin ( originally >>> written by Simon Busch ) which uses the file apns-conf.xml instead of >>> mbpi's serviceproviders.xml. >>> >>> There are a couple of issues we ran into... >>> >>> 1. How to represent APNs that support multiple usages types? >>> >>> The "type" attribute in apns-conf.xml is a list vs. the single type defined >>> by a gprs_context. >>> >>> The android-apndb plugin uses TYPE_INTERNET for APNs which support multiple >>> types. In order to properly support MMS, the core code needed to be >>> modified to allow the MMS properties to be additionally be set on a >>> TYPE_INTERNET context. Thus if an APN supports both Internet and MMS, our >>> DownloadManager can grab the additional MMS properties from the context and >>> handle MMS traffic. >>> >>> Now perhaps I missed something and there is a way to represent a combined >>> usage APN ( maybe using TYPE_ANY? ), but I couldn't see how to accomplish >>> without changes to the core gprs code. >>> >>> 2. No way to disable core ofono TYPE_MMS network config. >>> >>> The core gprs_context code has special logic for TYPE_MMS contexts which >>> configures the HTTP proxy using networking ioctl requests. We have an >>> external download manager that handles the actual download of content from >>> the message center. As it has logic to handle HTTP proxies already, if we >>> use TYPE_MMS, we'd need a way to disable ofono's builtin logic. >> >> you do realize that these are not actually standard HTTP proxies. You are >> suppose to talk to the MMS Proxy to reach the MMSC. That is how this works. >> You always go through the MMS Proxy. > > I haven't actually worked directly on the proxy support myself, and had > assumed this was a standard HTTP proxy. I will make sure my co-worker > handling the support in our download manager realizes this, and point him at > mmsd/connman's GWeb code for reference. > >> If you are mixing Internet proxies with MMS proxies, you end up in a really >> awkward configuration later down the road. > > Sorry, no intentions of mixing them. > >> As Denis mentioned we have been using this successfully within mmsd that we >> wrote. > > I saw that, we originally did a bunch of work with mmsd, however we decided > instead to add MMS capability to our ubuntu-download-manager used in Touch. feels weird if a download manager handles delivery receipts and read receipts. Especially since there are so many MMS specific header in the HTTP request. > > The current design of oFono works just fine by activating the > > MMS context and then talking to the MMS proxy to reach the MMSC. > > Sure, although as I mentioned orginally, if we already have an active data > connection and the apn happens to also support MMS, it seems odd not being > able to share. If you have an active data connection you can just compare the TYPE_MMS context details (APN, username, passed) with the TYPE_INTERNET context ones and if they match, you choose not to activate the MMS one and just use the Internet one. As I said in the other emails, oFono is activating contexts only on request. So either ConnMan requests the Internet context or mmsd requests the MMS context. oFono is not in the business of magically activating contexts. It only attaches to the GPRS part of the network. The only problem with the Internet context usage for MMS is that you now also have to talk to ConnMan to figure out when the IP part of the context is successfully configured. And here is where this gets complicated. And active context does not mean it is actively in use. You might actually use your WiFi and have an activated Internet context, but neither the IP or routing is set. Or in same case you are just on WiFi and have no Internet context activated. As you can see this can get extremely complex. We opted for just providing a MMS context type and an Internet context type. If the APN happens to be the same (not always the case btw.) then you just double activate that context. Send or receive your MMS and deactivate it. We have used this just fine. And in that case oFono does all the heavy lifting for you. You just need to talk to the MMS Proxy address that has been configured. That is it. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: RFC: Ubuntu Touch, MMS, and Provisioning
Hi Slava, >>> What do you think about extending org.ofono.ConnectionContext API with >>> Add/RemoveProperty calls and PropertyAdded/Removed signals to allow >>> platform/application specific properties of type string? Use cases >>> include marking connections as preferred/default or assigning priorities >>> to them, tracking the origin of the context (OTA, local provision or >>> manually edited) and who knows what else. >>> >> >> There are practical considerations that make this a bad idea, e.g. security >> concerns or the fact that our dbus bindings do not handle dynamic >> properties. Putting that aside... >> >> Your suggestion would be completely against our three core design values, >> e.g. "Minimal and Complete"; "Consistent" and "Easy to Use". >> >> I'm open to adding additional properties the "old way" if you can argue good >> use cases for them. > > A couple of examples. > > Suppose, we have a UI that allows the user to switch between "Automatic" and > "Custom" MMS or GPRS settings. One of the ways to implement that would be to > create a new context marked as "manual" and allow the user to edit it. The > old context remains but it's marked as "automatic" or whatever. Manual > context has a precedence over the automatic one. Switching back to automatic > means destroying the "manual" context or marking it as "disabled". All that > stuff gets saved to the ofono SIM-specific gprs file so that these context > properties don't get lost after swapping SIMs. > > Suppose, we have a different connection management system, which shows the > entire list of available access points to the user and allows to choose which > one to use. The selected context needs to be marked as "default" or > something. Again, this context property needs to survive SIM swap. With a > custom context property that's pretty easy to do. I am not even sure what’s your goal here? Are just trying to plain copy the horrible crap that Android puts in front of their users? You do realize that their list of APNs just purely exists since they never figured out on how to do the automatic provisioning properly and user friendly. I have to second Denis here that time is better spent in creating a proper provision database instead of just copying what AOSP provides. For example many operators are using SPN or IMSI prefixes to clearly identify their MNVO. We made the provisioning a plugin design for exactly that reason so that every product can provide its own database. You might also think this through once more. What does default actually mean. Can every context have a default tag? Which one do you pick then? Who is making this policy and who is ensuring what is active at what time. oFono is happily supporting multiple Internet contexts and have them all active. Our own modem hardware supports many contexts. We have run this with 4 Internet contexts at the same time (different and same ones). And I have to note that oFono never establishes contexts by itself. The only thing that oFono does is attach to the GPRS (packet switched) part of network. The APN establishment is a policy enforced by ConnMan for Internet contexts and mmsd for MMS contexts. The simplest approach we found with our devices is to only provide one Internet context from the user interface. If it is wrong for whatever reason or provisioning can not identify it, then the user gets asked once. If user wants to change it they can go through the APN setup process once more. Worked smoothly for us and is super user friendly. > Every project may have requirements that are more or less unique, often > influenced by the UI design. There's no need to push support for unique > project specific requirements into the common code, there's nothing here to > argue about. It's a matter of whether you want to make ofono a bit more > usable as is or you are fine with it being cloned and heavily modified for > each particular project. Do what we want or we fork your project. Nice :) Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: RFC: Ubuntu Touch, MMS, and Provisioning
Hi Tony, > We've been working on MMS support for Ubuntu Touch recently and have run into > a couple of stumbling blocks, so I have a few questions about the current MMS > logic in oFono ( we're still 1.12 based ), and in particular the > provisioning/management of gprs-contexts. > > As part of this work, we're planning to switch from using the builtin gprs > 'provision' plugin, to the new android-provision/apndb plugin ( originally > written by Simon Busch ) which uses the file apns-conf.xml instead of mbpi's > serviceproviders.xml. > > There are a couple of issues we ran into... > > 1. How to represent APNs that support multiple usages types? > > The "type" attribute in apns-conf.xml is a list vs. the single type defined > by a gprs_context. > > The android-apndb plugin uses TYPE_INTERNET for APNs which support multiple > types. In order to properly support MMS, the core code needed to be modified > to allow the MMS properties to be additionally be set on a TYPE_INTERNET > context. Thus if an APN supports both Internet and MMS, our DownloadManager > can grab the additional MMS properties from the context and handle MMS > traffic. > > Now perhaps I missed something and there is a way to represent a combined > usage APN ( maybe using TYPE_ANY? ), but I couldn't see how to accomplish > without changes to the core gprs code. > > 2. No way to disable core ofono TYPE_MMS network config. > > The core gprs_context code has special logic for TYPE_MMS contexts which > configures the HTTP proxy using networking ioctl requests. We have an > external download manager that handles the actual download of content from > the message center. As it has logic to handle HTTP proxies already, if we > use TYPE_MMS, we'd need a way to disable ofono's builtin logic. you do realize that these are not actually standard HTTP proxies. You are suppose to talk to the MMS Proxy to reach the MMSC. That is how this works. You always go through the MMS Proxy. If you are mixing Internet proxies with MMS proxies, you end up in a really awkward configuration later down the road. As Denis mentioned we have been using this successfully within mmsd that we wrote. The current design of oFono works just fine by activating the MMS context and then talking to the MMS proxy to reach the MMSC. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH] features: Describe Siri feature
Hi, >> --- >> doc/features.txt | 8 >> 1 file changed, 8 insertions(+) >> mode change 100644 => 100755 doc/features.txt >> > > Patch has been applied, thanks. can someone please STOP changing the mode on all files they touch. Seriously, this is so not cool. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH] hfp_hf_bluez5: Fix re-registering a modem for a device
Hi Johan, > Even for outgoing pairing requests we may receive the UUIDs property > changed after the device is paired and try to register it twice. > > The easiest way to reproduce this is when Extended Inquiry Response is > supported. > > When the device is paired, we receive the "Paired" PropertyChanged, > inside modem_register_from_proxy(), g_dbus_proxy_get_property() gets > the UUIDs extracted from the EIR data. Later, when the service > discovery is finished, the UUIDs property is re-sent and both may > contain the HFP AG UUID. My sources indicated to me that BlueZ should perform the SDP query first, and then signal Paired. Is this something we can not count on or is this an implementation issue inside BlueZ itself? >>> >>> BlueZ has always done pairing first and only then SDP. This is because >>> there are security mode 3 devices out there that do not permit any kind >>> of connection before pairing has been completed (i.e. even if we wanted >>> to do SDP first we can't with them). That said, we do at least delay the >>> NewConnection() callback until SDP has been completed. >> >> so when are we sending "Paired" property? We might have to delay that >> property update until SDP finished. > > Right now it's directly bound to when the device is actually paired, > i.e. ignoring any other ongoing operations such as SDP. > >> We have to make sure that we finished SDP after the pairing before >> updating any clients with property changes. > > I don't completely understand the rationale of wanting to make BlueZ's > clients so fragile. You could in theory get new services way after > pairing if this is configurable on the remote side. You could also get > the result of calling Device1.Connect() instead of Device1.Pair() in > BlueZ. > > On the other hand, we do delay the response to Device1.Pair() until SDP > is complete so delaying the Paired property would in a way be consistent > with that. I'll look into how simple this would be. We already track the > completion of SDP for each device internally with a svc_resolved boolean > so probably a new flag to indicate that "Paired" still needs to be > emitted should be all the extra context tracking we need. the pairing procedure is special and we should try to avoid any pointless round trips or wake ups for the clients here. We do know that we are running SDP after pairing anyway. So lets just wait for it to finish. Especially if we do that already anyway for Device1.Pair(). Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH] hfp_hf_bluez5: Fix re-registering a modem for a device
Hi Johan, >>> Even for outgoing pairing requests we may receive the UUIDs property >>> changed after the device is paired and try to register it twice. >>> >>> The easiest way to reproduce this is when Extended Inquiry Response is >>> supported. >>> >>> When the device is paired, we receive the "Paired" PropertyChanged, >>> inside modem_register_from_proxy(), g_dbus_proxy_get_property() gets >>> the UUIDs extracted from the EIR data. Later, when the service >>> discovery is finished, the UUIDs property is re-sent and both may >>> contain the HFP AG UUID. >> >> My sources indicated to me that BlueZ should perform the SDP query >> first, and then signal Paired. Is this something we can not count >> on or is this an implementation issue inside BlueZ itself? > > BlueZ has always done pairing first and only then SDP. This is because > there are security mode 3 devices out there that do not permit any kind > of connection before pairing has been completed (i.e. even if we wanted > to do SDP first we can't with them). That said, we do at least delay the > NewConnection() callback until SDP has been completed. so when are we sending "Paired" property? We might have to delay that property update until SDP finished. We have to make sure that we finished SDP after the pairing before updating any clients with property changes. Regards Marcel ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH v0 09/10] handsfree-audio: Add function to get hfp version
Hi Denis, This patch adds a new function to read the local supported Handsfree Profile version. --- include/handsfree-audio.h | 1 + src/handsfree-audio.c | 5 + 2 files changed, 6 insertions(+) diff --git a/include/handsfree-audio.h b/include/handsfree-audio.h index c5403c7..96597c2 100644 --- a/include/handsfree-audio.h +++ b/include/handsfree-audio.h @@ -34,6 +34,7 @@ struct ofono_handsfree_card *ofono_handsfree_card_create(const char *remote, const char *local); int ofono_handsfree_card_register(struct ofono_handsfree_card *card); void ofono_handsfree_card_remove(struct ofono_handsfree_card *card); +enum hfp_version ofono_handsfree_get_version(); >>> >>> I'd like to avoid such details in the card functionality. >>> >>> Are our possible scenarios the following? >>> >>> 1. SCO defer in kernel, CVSD mSBC supported >> >> yes. >> >>> 2. SCO defer not in kernel, CVSD mSBC supported >> >> not possible. Wide band speech requires defer setup to set the SCO >> parameters properly. >> >>> 3. SCO defer in kernel, CVSD supported >> >> yes. >> >>> 4. SCO defer not in kernel, CVSD supported >> >> yes. >> >>> >>> If so, can we always advertise HFP 1.6, but only declare support for CVSD in >>> case 2? >> >> I am not a spec (or qualification) expert, but I will try to expose >> my understanding of the docs: >> >> For case 2, my understanding is: if HF supports *only* CVSD >> (mandatory), it doesn't make sense to inform that codec negotiation is >> supported. HFP 1.5 (or older) shall not indicate that it supports >> codec negotiation (during service level connection negotiation) and it >> should not set the wide band speech support in the service record. >> > > There are other features of 1.6 though that might be useful. For example, > BTRH status is properly shown in CLCC, and a few other niceties. I see no > reason to fall back to 1.5 just because we do not support wide-band speech. > > Either way, the version decision is not up to the card, it is up to the > plugin. The function to get the version information needs to be dropped. > >> One thing that I can't answer is if it is allowed to inform in the >> service record that the HF supports HFP 1.6 (version), but don't set >> the wide band speech feature in record. >> > > I don't see why not. Both Codec Negotiation and Wide-band speech are labeled > optional for HF/AG in Table 3.1. The only note there says: "If Wide Band > Speech is supported, Codec Negotiation shall also be supported." However, > there is no note vice-versa. So my interpretation is that Codec Negotiation > can always be supported. Can we check the test spec? > > The problem I see is that by the time we register the SDP record, we still do > not know for sure whether the audio framework truly supports mSBC. so we want to expose HFP 1.6 and codec negogiation all the time. Even if mSBC codec is not available, because PA does not support it or is not running. I read the spec the same way that this fully spec compliant. If at some point in the future this changes, we can deal with that at that point. No need to over engineer it at this point. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH v0 8/9] hfp_hf_bluez5: Reject SCO if source doesn't match
Hi Claudio,
> This patch implements additional verification checking the Bluetooth
> source address instead of the remote address only.
> ---
> plugins/hfp_hf_bluez5.c | 64
> ++---
> 1 file changed, 45 insertions(+), 19 deletions(-)
>
> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> index d79647b..504b0f3 100644
> --- a/plugins/hfp_hf_bluez5.c
> +++ b/plugins/hfp_hf_bluez5.c
> @@ -60,6 +60,12 @@
> struct hfp {
> struct hfp_slc_info info;
> DBusMessage *msg;
> + char adapter[18];
> +};
> +
> +struct bt_peer {
> + char adapter[18];
> + char device[18];
> };
I do not get this. I am really not sure what this is all about and why
having Bluetooth addresses as strings here is a good idea.
>
> static GHashTable *modem_hash = NULL;
> @@ -70,11 +76,18 @@ static guint sco_watch = 0;
> static gboolean modem_address_cmp(gpointer key, gpointer value,
> gpointer user_data)
> {
> - const char *dst = user_data;
> + const struct bt_peer *bt_peer = user_data;
> struct ofono_modem *modem = value;
> - const char *address = ofono_modem_get_string(modem, "Address");
> + const char *device = ofono_modem_get_string(modem, "Address");
> + struct hfp *hfp;
> + gboolean ret;
>
> - return g_strcmp0(address, dst) != 0 ? FALSE : TRUE;
> + ret = g_strcmp0(device, bt_peer->device);
> + if (ret != 0)
> + return FALSE;
> +
> + hfp = ofono_modem_get_data(modem);
> + return g_strcmp0(hfp->adapter, bt_peer->adapter) != 0 ? FALSE : TRUE;
> }
>
> static void hfp_debug(const char *str, void *user_data)
> @@ -284,14 +297,14 @@ static DBusMessage
> *profile_new_connection(DBusConnection *conn,
> {
> struct hfp *hfp;
> struct ofono_modem *modem;
> + struct sockaddr_rc src, dst;
> DBusMessageIter iter;
> GDBusProxy *proxy;
> DBusMessageIter entry;
> - const char *device, *alias, *address;
> + const char *device, *alias;
> + char device_address[18];
> int fd, err;
>
> - DBG("Profile handler NewConnection");
> -
> if (dbus_message_iter_init(msg, &entry) == FALSE)
> goto invalid;
>
> @@ -310,11 +323,6 @@ static DBusMessage
> *profile_new_connection(DBusConnection *conn,
>
> dbus_message_iter_get_basic(&iter, &alias);
>
> - if (g_dbus_proxy_get_property(proxy, "Address", &iter) == FALSE)
> - goto invalid;
> -
> - dbus_message_iter_get_basic(&iter, &address);
> -
> dbus_message_iter_next(&entry);
> if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
> goto invalid;
> @@ -323,7 +331,13 @@ static DBusMessage
> *profile_new_connection(DBusConnection *conn,
> if (fd < 0)
> goto invalid;
>
> - modem = modem_register(device, address, alias);
> + if (bt_getsockpeers(fd, (struct sockaddr *) &src,
> + (struct sockaddr *) &dst,
> + sizeof(struct sockaddr_rc)) < 0)
> + goto invalid;
> +
> + bt_ba2str(&dst.rc_bdaddr, device_address);
> + modem = modem_register(device, device_address, alias);
> if (modem == NULL) {
> close(fd);
> return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
> @@ -339,6 +353,9 @@ static DBusMessage *profile_new_connection(DBusConnection
> *conn,
>
> hfp = ofono_modem_get_data(modem);
> hfp->msg = dbus_message_ref(msg);
> + bt_ba2str(&src.rc_bdaddr, hfp->adapter);
> +
> + DBG("Profile handler NewConnection: %s", device_address);
>
> return NULL;
>
> @@ -394,9 +411,9 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition
> cond,
> gpointer user_data)
> {
> struct ofono_modem *modem;
> - struct sockaddr_sco saddr;
> + struct sockaddr_sco src, dst;
> + struct bt_peer bt_peer;
> socklen_t optlen;
> - char dst[18];
> int sk, nsk;
>
> if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
> @@ -404,15 +421,24 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition
> cond,
>
> sk = g_io_channel_unix_get_fd(io);
>
> - memset(&saddr, 0, sizeof(saddr));
> - optlen = sizeof(saddr);
> + memset(&dst, 0, sizeof(dst));
> + optlen = sizeof(dst);
>
> - nsk = accept(sk, (struct sockaddr *) &saddr, &optlen);
> + nsk = accept(sk, (struct sockaddr *) &dst, &optlen);
> if (nsk < 0)
> return TRUE;
>
> - bt_ba2str(&saddr.sco_bdaddr, dst);
> - modem = g_hash_table_find(modem_hash, modem_address_cmp, dst);
> + if (bt_getsockpeers(nsk, (struct sockaddr *) &src, NULL, optlen) < 0) {
> + close(nsk);
> + return TRUE;
> + }
> +
> + bt_ba2str(&src.sco_bdaddr, bt_peer.adapter);
> + bt_ba2str(&dst.sco_bdaddr, bt_peer.device);
> +
> + DBG("SCO:
Re: [PATCH v0 3/9] hfp_hf_bluez5: Add SCO listen socket
Hi Claudio,
> This patch adds the initial SCO server socket handling. BtIO and BlueZ
> functions should not be used, with the new Profile API the objetive is
> to get rid of these dependencies.
> ---
> plugins/hfp_hf_bluez5.c | 93
> +
> 1 file changed, 93 insertions(+)
>
> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> index 0e496ee..398dee0 100644
> --- a/plugins/hfp_hf_bluez5.c
> +++ b/plugins/hfp_hf_bluez5.c
> @@ -64,6 +64,7 @@ struct hfp {
> static GHashTable *modem_hash = NULL;
> static GHashTable *devices_proxies = NULL;
> static GDBusClient *bluez = NULL;
> +static guint sco_watch = 0;
>
> static void hfp_debug(const char *str, void *user_data)
> {
> @@ -378,6 +379,89 @@ static const GDBusMethodTable profile_methods[] = {
> { }
> };
>
> +static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
> + gpointer user_data)
> +{
> + struct sockaddr_sco saddr;
> + socklen_t optlen;
> + int sk, nsk;
> +
> + if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
> + return FALSE;
> +
> + sk = g_io_channel_unix_get_fd(io);
> +
> + memset(&saddr, 0, sizeof(saddr));
> + optlen = sizeof(saddr);
> +
> + nsk = accept(sk, (struct sockaddr *) &saddr, &optlen);
> + if (nsk < 0)
> + return TRUE;
> +
> + /* TODO: Verify if the device has a modem */
> +
> + return TRUE;
> +}
> +
> +static GIOChannel *sco_listen(int *err)
> +{
I do not like this int *err. Why is this a separate function anyway.
Just do the whole SCO server socket setup with accept watch setup in one
function. There is no advantage in having two functions.
> + struct sockaddr_sco addr;
> + GIOChannel *io;
> + int sk, defer_setup = 1;
> +
> + sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_SCO);
> + if (sk < 0) {
> + *err = -errno;
> + return NULL;
> + }
> +
> + /* Bind to local address */
> + memset(&addr, 0, sizeof(addr));
> + addr.sco_family = AF_BLUETOOTH;
> + bt_bacpy(&addr.sco_bdaddr, BDADDR_ANY);
> +
> + if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
> + close(sk);
> + *err = -errno;
> + return NULL;
> + }
> +
> + if (setsockopt(sk, SOL_BLUETOOTH, BT_DEFER_SETUP,
> + &defer_setup, sizeof(defer_setup)) < 0) {
> + ofono_warn("Can't enable deferred setup: %s (%d)",
> + strerror(errno), errno);
> + *err = -errno;
This is semantically incorrect. You can not return non-NULL and an
error.
Can we actually make HFP 1.6 work properly without defer setup enabled?
> + }
> +
> + io = g_io_channel_unix_new(sk);
> + g_io_channel_set_close_on_unref(io, TRUE);
> + g_io_channel_set_flags(io, G_IO_FLAG_NONBLOCK, NULL);
Can you just open the SOCK_NONBLOCK and SOCK_CLOEXEC as we should be
always doing.
> +
> + if (listen(sk, 5) < 0) {
> + g_io_channel_unref(io);
> + *err = -errno;
> + return NULL;
> + }
> +
> + return io;
> +}
> +
> +static int sco_init(void)
> +{
> + GIOCondition cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
What is this extra declaration good for? I rather have it inside the
call that is adding the watch.
> + GIOChannel *sco_io;
> + int err = 0;
> +
> + sco_io = sco_listen(&err);
> + if (sco_io == NULL)
> + return err;
> +
> + sco_watch = g_io_add_watch(sco_io, cond, sco_accept, NULL);
> + g_io_channel_unref(sco_io);
> +
> + return 0;
> +}
> +
> static void connect_handler(DBusConnection *conn, void *user_data)
> {
> DBG("Registering External Profile handler ...");
> @@ -500,6 +584,12 @@ static int hfp_init(void)
> if (DBUS_TYPE_UNIX_FD < 0)
> return -EBADF;
>
> + err = sco_init();
> + if (err < 0) {
> + ofono_error("SCO: %s(%d)", strerror(-err), -err);
> + return err;
> + }
> +
> /* Registers External Profile handler */
> if (!g_dbus_register_interface(conn, HFP_EXT_PROFILE_PATH,
> BLUEZ_PROFILE_INTERFACE,
> @@ -550,6 +640,9 @@ static void hfp_exit(void)
>
> g_hash_table_destroy(modem_hash);
> g_hash_table_destroy(devices_proxies);
> +
> + if (sco_watch)
Normally we do sco_watch > 0 for the GLib sources.
> + g_source_remove(sco_watch);
> }
>
> OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin",
> VERSION,
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH v0 5/9] bluez5: Add bt_getsockpeers()
Hi Claudio,
> This patch adds a generic Bluetooth helper function to allow getting
> the adapter and device Bluetooth Address from Bluetooth sockets.
> ---
> plugins/bluez5.c | 22 ++
> plugins/bluez5.h | 3 +++
> 2 files changed, 25 insertions(+)
>
> diff --git a/plugins/bluez5.c b/plugins/bluez5.c
> index d7e85f2..06629e1 100644
> --- a/plugins/bluez5.c
> +++ b/plugins/bluez5.c
> @@ -52,6 +52,28 @@ int bt_ba2str(const bdaddr_t *ba, char *str)
> ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]);
> }
>
> +int bt_getsockpeers(int sock, struct sockaddr *src, struct sockaddr *dst,
> + socklen_t len)
> +{
I really do not get why this is helpful at all. Except for making the
code less readable. So please do not do it. Call the socket functions
directly from where they are used.
In addition the name of this function is totally misleading. It does not
describe at all what it does. The peer is always the remote address. You
are not getting the src and dst peers here. That is not how the socket
nomenclature works. It is either socket name for the local address or
peer name for the remote address.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/3] build: Use AM_CPPFLAGS instead of INCLUDES
Hi Lucas, > Makefile.am:518: warning: 'INCLUDES' is the old name for 'AM_CPPFLAGS' (or > '*_CPPFLAGS') > --- > Makefile.am | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/3] build: Do not use deprecated AM_CONFIG_HEADER
Hi Lucas, > The long-obsoleted AM_CONFIG_HEADER macro was removed in automake 1.13. > Use AC_CONFIG_HEADERS instead. > --- > configure.ac | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/3] storage: Include sys/types.h for ssize_t
Hi Lucas, > src/storage.h:32:1: error: unknown type name 'ssize_t' > src/storage.h:36:1: error: unknown type name 'ssize_t' > --- > src/storage.h | 1 + > 1 file changed, 1 insertion(+) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH v1 1/6] Makefile: Add configure option for BlueZ 4 and 5
Hi Claudio,
> BlueZ 5 is the default option. If --enable-bluez4 is provided, oFono
> plugins based on BlueZ 4 are enabled prior to BlueZ 5.
> --disable-bluetooth configure option disables BlueZ 5 and BlueZ 4
> plugins.
> ---
> Makefile.am | 4 +++-
> configure.ac | 24
> 2 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 0d2ba9f..811a3f5 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -407,7 +407,7 @@ builtin_sources += plugins/samsung.c
> builtin_modules += sim900
> builtin_sources += plugins/sim900.c
>
> -if BLUETOOTH
> +if BLUEZ4
> builtin_modules += bluetooth
> builtin_sources += plugins/bluetooth.c plugins/bluetooth.h
>
> @@ -742,6 +742,7 @@ tools_stktest_LDADD = @GLIB_LIBS@ @DBUS_LIBS@
> endif
> endif
>
> +if BLUEZ4
> if DUNDEE
> sbin_PROGRAMS += dundee/dundee
>
> @@ -759,6 +760,7 @@ if SYSTEMD
> systemdunit_DATA += dundee/dundee.service
> endif
> endif
> +endif
>
> endif
>
> diff --git a/configure.ac b/configure.ac
> index 450352b..47b0600 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -173,15 +173,23 @@ AC_ARG_ENABLE(qmimodem,
> AC_HELP_STRING([--disable-qmimodem],
> AM_CONDITIONAL(QMIMODEM, test "${enable_qmimodem}" != "no")
>
> AC_ARG_ENABLE(bluetooth, AC_HELP_STRING([--disable-bluetooth],
> - [disable Bluetooth modem support]),
> - [enable_bluetooth=${enableval}])
> -if (test "${enable_bluetooth}" != "no"); then
> - PKG_CHECK_MODULES(BLUEZ, bluez >= 4.99, dummy=yes,
> - AC_MSG_ERROR(Bluetooth library >= 4.99 is required))
> + [disable BlueZ 4 and BlueZ 5 plugins support]),
> + [enable_bluez5=${enableval}])
> +
> +AC_ARG_ENABLE(bluez4, AC_HELP_STRING([--enable-bluez4],
> + [enable BlueZ 4 plugins support prior to BlueZ
> 5]),
> + [enable_bluez4=${enableval}])
> +
> +if (test "${enable_bluez4}" = "yes"); then
> + PKG_CHECK_MODULES(BLUEZ, bluez >= 4.99 bluez < 5,
> + enable_bluez4=yes enable_bluez5=no,
> + AC_MSG_ERROR(Bluetooth library >= 4.99 and < 5 is
> required))
> + AC_SUBST(BLUEZ_CFLAGS)
> + AC_SUBST(BLUEZ_LIBS)
> fi
> -AC_SUBST(BLUEZ_CFLAGS)
> -AC_SUBST(BLUEZ_LIBS)
> -AM_CONDITIONAL(BLUETOOTH, test "${enable_bluetooth}" != "no")
> +
> +AM_CONDITIONAL(BLUEZ4, test "${enable_bluez4}" = "yes")
> +AM_CONDITIONAL(BLUEZ5, test "${enable_bluez5}" != "no" && test
> "${enable_bluez4}" != "yes")
can we keep BLUETOOTH conditional for overall Bluetooth enabled/disabled
and just make BLUEZ4 the if-else statement for selecting which one.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: tun interface name conflicts with ppp driver
Hi Cedric, > > actually we are using ppp%d syntax to assign the network interfaces. So > > the kernel does the numbering. This is a bug in pppd and not ours. > > > > Looking at the pppd code, it seems to hardcodes ppp0. So please complain > > to them for assuming that a certain device name is owned by them. > > > > pppd requests the ppp driver to create a network interface with the ioctl > PPPIOCNEWUNIT. If the parameter passed to the ioctl is -1, the ppp driver > will allocate the unit itself. Because it's the first network interface > created by the ppp driver, and the ppp driver doesn't know about the tun > interface, it's choosing 0. Check ppp_create_interface() in ppp_generic.c > in the kernel to see how this is done. > > So it seems the ppp driver reserves the ppp%d interface names. that is not how the kernel works. It is global and tun interface will not interfere with ppp interface. In case ppp driver and pppd would do it right, then there would be no conflict. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: tun interface name conflicts with ppp driver
Hi Cedric, > when a context is active, the interface name is typically ppp0. If you try > to run pppd for some other device/interface, the kernel ppp driver prints > an error because it can't register ppp0. It looks like it's assumed that > ppp* interface names are reserved for the ppp_driver. > Can the tun interface name be changed to eg. ofono0? actually we are using ppp%d syntax to assign the network interfaces. So the kernel does the numbering. This is a bug in pppd and not ours. Looking at the pppd code, it seems to hardcodes ppp0. So please complain to them for assuming that a certain device name is owned by them. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] atmodem: Probe device for previous APN.
Hi Denis, > >>> If the device has retained parameters for a previously defined IP > >>> context, is is probed via AT+CGDCONT?. > >> > >> this is really not a good idea. The AT+CGDCONT? settings are on a per > >> device basis. That is not how oFono actually works. It operates on a per > >> SIM card basis. And since you do not know to what SIM card these > >> previous settings apply to, the only sensible thing to do is to ignore > >> them. > > > > Okay, I was not aware of this. But surely the common case (i.e. the only > > one > > I've personally seen) is to have a single SIM card per device, and under > > those > > limited circumstances it is safe to assume that the settings are valid for > > that > > SIM. Would it be reasonable to implement such a fallback for this more > > limited > > case? > > > > I admit your approach did make me chuckle for being so unorthodox :) > However, I think your perception is slightly wrong, there are people who > switch sim cards like crazy (e.g. frequent travelers) so this form of > provisioning is not really a good idea in those cases. even if you switch your SIM card only once, it is a bad idea to start out with a wrong context. Some operators do not include actually domain names and a flatrate in one could mean pay as you go in another and drain your bank account. Most operators should have their backends for different billings fixed, but you never know. It is really the only sensible way to bind the APN settings to your SIM card. Why the SIM card does not come with the APN settings stored is another story. You need to blame the operators for that. > >> You do know that oFono does keep its settings per SIM card persistent. > >> So no matter what device you enter that SIM card into, the settings will > >> be available the next time you try to connect. So it is a one time > >> configuration option. > > > > Yes, I do know this. But it's just not a great fit for our use case. > > > > We're using ConnMan and oFono to manage networking for computing appliances > > (e.g. interactive kiosks and digital signs). These are typically deployed > > in > > fleets of anywhere from 10 to 10,000 units. Clearly we do not want to > > manually > > configure an APN for each machine. ;) > > > > Ideally, we try to use a single configuration for the entire fleet to keep > > the > > fleet easy to manage. But for a variety of business and technical reasons, > > multiple service providers may be used within a given fleet. So we can't > > simply > > provision a single APN for the entire fleet. What we really want is to be > > able > > to plug a modem into the machine and it will Just Work. > > > > So we'd like to choose the right APN based solely on information from the > > modem. > > I've observed that modems tend to arrive with an IP context pre-configured > > (including the correct APN). I assume the service provider sets this as > > part of > > some internal provisioning process. Perhaps it is not safe to assume that > > this > > will be the case with the majority of devices and/or providers. On the > > other > > hand, if it is, it seems sensible to make use of that data. > > > > The alternative to pulling the data from the device is to provide a > > mechanism > > for specifying the APN policy for a particular fleet. (At least in theory > > the > > APN selection policy can vary from one fleet to the next.) I assume this > > would > > be implemented as a custom provisioning plugin that either hard-codes the > > policy > > for the fleet, or provides a configuration mechanism that is flexible > > enough to > > accommodate the needs of most fleets (probably using a provider -> APN > > mapping, > > assuming we can guess the provider e.g. using the > > mobile-broadband-provider-info > > database). > > I'm getting lost, why is the default mobile-broadband-provider-info > plugin not good enough? And why don't you simply create your own > database based on the providers you are targeting? To me it seems like > you need X entries for all X providers you have. Unless the settings > vary within a given provider somehow? The default mobile-broadband-provider-info database has too many duplicate choices and in that case oFono can not auto-provision your SIM card, but a specific smaller version of that database just target to your fleet would make oFono auto-provision the settings and you get exactly what you wanted in the first place. No extra code to be written. It is really a database problem that oFono backs out with the auto-provision in most cases. That database is just overloaded. And in case of oFono provision you also get the SPN information of your provider if they provided it. I would trust the combination of MCC, MNC and SPN more than anything stored via AT+CGDCONT. In addition you can write your own provisioning plugin in case you do not like the XML format from mobile-broadband-provider-info. Regards Marcel ___
Re: [PATCH 2/2] atmodem: Probe device for previous APN.
Hi Forest, > If the device has retained parameters for a previously defined IP > context, is is probed via AT+CGDCONT?. this is really not a good idea. The AT+CGDCONT? settings are on a per device basis. That is not how oFono actually works. It operates on a per SIM card basis. And since you do not know to what SIM card these previous settings apply to, the only sensible thing to do is to ignore them. You do know that oFono does keep its settings per SIM card persistent. So no matter what device you enter that SIM card into, the settings will be available the next time you try to connect. So it is a one time configuration option. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/1] telit: stay 'online' until POST_SIM state reached
Hi Jonas,
> In offline state where CFUN=4, the Telit HE910 also powers down the SIM
> card so AT commands that query the SIM will fail. These failures result
> in ofono not getting to POST_SIM state where it will export the GPRS
> feature.
>
> This patch changes the Telit driver so it will not go immediately
> to CFUN=4 after enable, but to wait for the post_sim state to be
> reached before doing so. In addition, the HE910 might send QSS: 0
> unsolicited reports while in CFUN=4 state... this patch makes it so
> that these are ignored until the modem is onlined and the actual
> SIM state can be queried again.
> ---
>
> This patch has been tested with a Telit HE910 and it works fine there. It
> would be great if someone with other Telit modems could check if
> this works with those models or whether we need to wrap some of this with
> checks for model type, firmware version, etc...
>
> Thanks,
> Jonas
>
>
> plugins/telit.c | 64
> +++
> 1 file changed, 60 insertions(+), 4 deletions(-)
>
> diff --git a/plugins/telit.c b/plugins/telit.c
> index fe2ccd6..a0f7deb 100644
> --- a/plugins/telit.c
> +++ b/plugins/telit.c
> @@ -62,6 +62,7 @@
>
> static const char *none_prefix[] = { NULL };
> static const char *rsen_prefix[]= { "#RSEN:", NULL };
> +static const char *qss_prefix[] = { "#QSS:", NULL };
>
> struct telit_data {
> GAtChat *chat; /* AT chat */
> @@ -219,7 +220,7 @@ static void switch_sim_state_status(struct ofono_modem
> *modem, int status)
>
> switch (status) {
> case 0: /* SIM not inserted */
> - if (data->have_sim == TRUE) {
> + if (data->have_sim == TRUE && ofono_modem_get_online(modem)) {
> ofono_sim_inserted_notify(data->sim, FALSE);
> data->have_sim = FALSE;
> data->sms_phonebook_added = FALSE;
> @@ -233,6 +234,14 @@ static void switch_sim_state_status(struct ofono_modem
> *modem, int status)
> }
> break;
> case 3: /* SIM inserted, SMS and phonebook ready */
> + /* It's possible that we arrive at QSS=3 state without
> + * ever seeing QSS=2, so we need to make sure that we've
> + * also done the QSS=2 work, as well
> + */
> + if (data->have_sim == FALSE) {
> + ofono_sim_inserted_notify(data->sim, TRUE);
> + data->have_sim = TRUE;
> + }
> if (data->sms_phonebook_added == FALSE) {
> ofono_phonebook_create(modem, 0, "atmodem", data->chat);
> ofono_sms_create(modem, 0, "atmodem", data->chat);
I don't remember how this made it upstream. The SMS and Phonebook atom
drivers should have vendor quirks to not register until the SIM is fully
ready. We do that for IFX for Phonebook and I was going to add another
one for IFX and SMS handling. Telit should do the same.
I am surprised adding atoms outside of pre_sim, post_sim and post_online
works as it should. Since that clearly has never been fully tested. And
is not our recommended way of adding atoms.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: Ofono Modem Service acces
Hi Rohit, > How ofono access multiple service of the modems. Is ofono work with > modem only on AT commands or access the service of modem through AT > commands? it can use 3GPP 27.007, 27.010 or vendor protocols like Phonet/ISI, Qualcomm QMI or others. > What is this qmi interface. Please provide me answer. I have confusion on > it. You might want to ask Qualcomm this ;) Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: qmi on huawei
Hi Cedric, > > The ofono.git HEAD already sets the driver, but the qmi and net > > detection is not always correct. So just overwrite the values for the > > device nodes and see if that works for you. > > > > Thanks for the suggestion, i started on this before this change was > commited. > I also had the issue to detect qmi, because udev doesn't export the usb > properties for cdc-wdm devices. > The patch i did for this, is check ID_VENDOR_ID and ID_MODEL_ID of > usb_device instead of device in check_usb_devices(). > > Using the gobi driver for the E398, it looks like there's a memory leak of > qmi_service when plugging/unplugging the device. that might be possible. Can you give me some valgrind traces or similar so I can see where this might be coming from. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: qmi on huawei
Hi Cedric,
> I have a Huawei E398 dongle with a qmi interface, and i'm trying to make
> ofono use the qmi interface for the gprs-context atom. I have this working,
> but depending on how i unref the qmi_device in huawei_disable, ofono
> crashes or there's have a memory leak.
>
> The first approach i've tried is the same as in the gobi plugin.
> I call qmi_device_shutdown() from huawei_disable(), and unref the
> qmi_device from the callback.
> The problem with this is: when i unplug the device ofono crashes.
> When the device is unplugged, first huawei_disable() is called, then
> huawei_remove(), and
> afterwards shutdown_cb() is called. At that time the ofono_modem struct is
> already freed, shutdown_cb() tries to access it and crashes.
>
> The second approach i've tried is to unref the qmi_device directly from
> huawei_disable.
> With this approach, i see a memory leak of the qmi_service.
> qmi_gprs_context_remove() is being called from flush_atoms(). And i can see
> release_client() is being called. But the service_release_callback(), which
> should free the service, is never being called.
> I think the request get's canceled because qmi_device_unref() is being
> called from huawei_disable(). The request is destroyed and the callback
> which should free the qmi_service is never called.
>
> Does anyone have thoughts/ideas on how i can fix this?
actually what I am doing with the E398 is just to switch the driver to
"gobi" instead of "huawei". For testing you can just hardcode it.
diff --git a/plugins/udevng.c b/plugins/udevng.c
index afb02ca..53b12b3 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -338,6 +338,9 @@ static gboolean setup_huawei(struct modem_info *modem)
}
}
+ qmi = "/dev/cdc-wdm0";
+ net = "wwan0";
+
if (qmi != NULL && net != NULL) {
ofono_modem_set_driver(modem->modem, "gobi");
goto done;
The ofono.git HEAD already sets the driver, but the qmi and net
detection is not always correct. So just overwrite the values for the
device nodes and see if that works for you.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH] systemd: prevent duplicate logging messages in journal
Hi Marti, > By default, both stderr and syslog messages go to the systemd journal, > which results in duplicate messages being logged. > > Thanks to Vinicius Costa Gomes for pointing out this problem. > --- > dundee/dundee.service.in | 1 + > src/ofono.service.in | 1 + > 2 files changed, 2 insertions(+) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [RFC] Multi-recipient SMS implementation proposals
Hi Guillaume,
> 2-Modify existing SendMessage API in adding a new mms argument:
> GDBUS_ASYNC_METHOD("SendMessage",
> GDBUS_ARGS({ "to", "s" }, { "text", "s" }, { "mms", "i" }),
> GDBUS_ARGS({ "path", "o" }),
> sms_send_message)
oFono 1.x has a stable API. Why are you proposing to break the API?
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [RFC] gatchat: Print error message if opening tun failes
Hi Daniel,
> This is a very common misstake. Let's help the users to
> configure their system correctly.
> ---
> gatchat/ppp_net.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/gatchat/ppp_net.c b/gatchat/ppp_net.c
> index 1609b99..7082de7 100644
> --- a/gatchat/ppp_net.c
> +++ b/gatchat/ppp_net.c
> @@ -155,8 +155,12 @@ struct ppp_net *ppp_net_new(GAtPPP *ppp, int fd)
> if (fd < 0) {
> /* open a tun interface */
> fd = open("/dev/net/tun", O_RDWR);
> - if (fd < 0)
> + if (fd < 0) {
> + fprintf(stderr, "Couldn't open tun device. "
> + "Do you run oFono as root and do you "
> + "have the TUN module loaded?");
really, stderr?
> goto error;
> + }
>
> ifr.ifr_flags = IFF_TUN | IFF_NO_PI;
> strcpy(ifr.ifr_name, "ppp%d");
Aren't the atom drivers checking this case for you?
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH] doc: Trivial whitespace fixes
Hi Mikel, > Always use tabs instead of spaces for indentation in documentation > files. > --- > doc/location-reporting-api.txt |2 +- > doc/voicecallmanager-api.txt | 12 ++-- > 2 files changed, 7 insertions(+), 7 deletions(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH_v6] test: Add hangup-call script for GCF testing
Hi Guillaume, > Makefile.am |3 ++- > test/hangup-call | 14 ++ > 2 files changed, 16 insertions(+), 1 deletions(-) > create mode 100755 test/hangup-call patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH_v5] test: Add hangup-call script for GCF testing
Hi Guillaume, > Makefile.am |3 ++- > test/hangup-call | 14 ++ > 2 files changed, 16 insertions(+), 1 deletions(-) > create mode 100755 test/hangup-call Applying: test: Add hangup-call script for GCF testing error: patch failed: Makefile.am:625 error: Makefile.am: patch does not apply Patch failed at 0001 test: Add hangup-call script for GCF testing Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH_v4] test: Add hangup-call script for GCF testing
Hi Guillaume,
> Makefile.am |3 ++-
> test/hangup-call | 28
> 2 files changed, 30 insertions(+), 1 deletions(-)
> create mode 100755 test/hangup-call
>
> diff --git a/Makefile.am b/Makefile.am
> index 7e8f12c..8cf6920 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -625,7 +625,8 @@ test_scripts = test/backtrace \
> test/swap-calls \
> test/release-and-answer \
> test/hold-and-answer \
> - test/hangup-multiparty
> + test/hangup-multiparty \
> + test/hangup-call
>
> if TEST
> testdir = $(pkglibdir)/test
> diff --git a/test/hangup-call b/test/hangup-call
> new file mode 100755
> index 000..0765919
> --- /dev/null
> +++ b/test/hangup-call
> @@ -0,0 +1,28 @@
> +#!/usr/bin/python
> +
> +import sys
> +import dbus
> +
> +bus = dbus.SystemBus()
> +
> +manager = dbus.Interface(bus.get_object('org.ofono', '/'),
> + 'org.ofono.Manager')
> +
> +modems = manager.GetModems()
> +path = modems[0][0]
> +
> +manager = dbus.Interface(bus.get_object('org.ofono', path),
> + 'org.ofono.VoiceCallManager')
> +
> +calls = manager.GetCalls()
> +if (len(calls) == 0):
> + print "No calls available"
> + sys.exit(1)
these are test scripts. So just remove the "no calls" check. I rather
see a D-Bus exception than trying to work around it.
> +
> +if (len(sys.argv) < 2):
> + print "Usage: %s [ Call Path ]" % (sys.argv[0])
> + sys.exit(1)
> +
> +call = dbus.Interface(bus.get_object('org.ofono', sys.argv[1]),
> + 'org.ofono.VoiceCall')
> +call.Hangup()
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH_v3 2/2] test: Add hangup-call script for GCF testing
Hi Guillaume,
> Makefile.am |3 ++-
> test/hangup-call | 36
> 2 files changed, 38 insertions(+), 1 deletions(-)
> create mode 100755 test/hangup-call
>
> diff --git a/Makefile.am b/Makefile.am
> index 7e8f12c..8cf6920 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -625,7 +625,8 @@ test_scripts = test/backtrace \
> test/swap-calls \
> test/release-and-answer \
> test/hold-and-answer \
> - test/hangup-multiparty
> + test/hangup-multiparty \
> + test/hangup-call
>
> if TEST
> testdir = $(pkglibdir)/test
> diff --git a/test/hangup-call b/test/hangup-call
> new file mode 100755
> index 000..bc77acf
> --- /dev/null
> +++ b/test/hangup-call
> @@ -0,0 +1,36 @@
> +#!/usr/bin/python
> +
> +import sys
> +import dbus
> +
> +bus = dbus.SystemBus()
> +
> +manager = dbus.Interface(bus.get_object('org.ofono', '/'),
> + 'org.ofono.Manager')
> +
> +modems = manager.GetModems()
> +path = modems[0][0]
> +
> +manager = dbus.Interface(bus.get_object('org.ofono', path),
> + 'org.ofono.VoiceCallManager')
> +
> +calls = manager.GetCalls()
> +if (len(calls) == 0):
> + print "No calls available"
> + sys.exit(1)
> +
> +if (len(sys.argv) < 2):
> + i = 0
> + for path, properties in calls:
> + multi = properties["Multiparty"]
> + print "Call Path [ %s ] Multiparty %d" % (path, multi)
> + print
> + i += 1
> +
> + print "Usage: %s [ Call Path ]" % (sys.argv[0])
> + print
> + sys.exit(1)
we have list-calls for this. So why not make this really simple and and
require the call object path as input.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH_v3 1/2] test: Add hangup-multiparty script for GCF testing
Hi Guillaume, > Makefile.am|3 ++- > test/hangup-multiparty | 20 > 2 files changed, 22 insertions(+), 1 deletions(-) > create mode 100755 test/hangup-multiparty patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH_v3 3/3] test: Add hangup-call script for GCF testing
Hi Guillaume,
> >> Makefile.am |3 ++-
> >> test/hangup-call | 39 +++
> >> 2 files changed, 41 insertions(+), 1 deletions(-)
> >> create mode 100755 test/hangup-call
> >>
> >> diff --git a/Makefile.am b/Makefile.am
> >> index a689483..534245e 100644
> >> --- a/Makefile.am
> >> +++ b/Makefile.am
> >> @@ -624,7 +624,8 @@ test_scripts = test/backtrace \
> >>test/swap-calls \
> >>test/release-and-answer \
> >>test/hold-and-answer \
> >> - test/hangup-call-state
> >> + test/hangup-call-state \
> >> + test/hangup-call
> >>
> >> if TEST
> >> testdir = $(pkglibdir)/test
> >> diff --git a/test/hangup-call b/test/hangup-call
> >> new file mode 100755
> >> index 000..926efc4
> >> --- /dev/null
> >> +++ b/test/hangup-call
> >> @@ -0,0 +1,39 @@
> >> +#!/usr/bin/python
> >> +
> >> +import sys
> >> +import dbus
> >> +
> >> +bus = dbus.SystemBus()
> >> +
> >> +manager = dbus.Interface(bus.get_object('org.ofono', '/'),
> >> + 'org.ofono.Manager')
> >> +
> >> +modems = manager.GetModems()
> >> +path = modems[0][0]
> >> +
> >> +manager = dbus.Interface(bus.get_object('org.ofono', path),
> >> + 'org.ofono.VoiceCallManager')
> >> +
> >> +calls = manager.GetCalls()
> >> +if (len(calls) == 0):
> >> + print "No calls available"
> >> + sys.exit(1)
> >> +
> >> +if (len(sys.argv) < 2):
> >> + i = 0
> >> + for path, properties in calls:
> >> + multi = properties["Multiparty"]
> >> + print "Call ID [ %d ] Multiparty %d" % (i, multi)
> >> + print
> >> + i += 1
> >> +
> >> + print "Usage: %s [ Call ID ]" % (sys.argv[0])
> >> + print
> >> + sys.exit(1)
> >> +
> >> +path = calls[int(sys.argv[1])][0]
> >> +
> >> +call = dbus.Interface(bus.get_object('org.ofono', path),
> >> + 'org.ofono.VoiceCall')
> >> +
> >> +call.Hangup()
> > why are we not just asking to provide the object path as argument and
> > then list-calls can be used to determine which call to hang up. That way
> > also the hangup-call-state hack is not needed.
> >
> > Using the object path also makes this less racy since we identify the
> > call by object path and not a random number that might no longer be
> > valid.
>
> We can use voicecall path indeed, however this script will not be enough
> to clear all the remote parties of a multiparty call at once.
> For instance, I have a 3GPP test case that ask me to do this:
> - create a multiparty call with 3 parties
> - create a new single call
> - clear multiparty call in held
>
> I could either use the hangup-call-state script (with held argument)
> either create a new script using HangupMultiparty() method from
> voicecallmanager API (like there is a hangup-all script).
> One of those scripts would avoid to call thrice hangup-call for a
> multiparty call with 3 party for instance.
sounds to me that hangup-multiparty script is a good idea.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH_v3 1/3] test: Add hold-and-answer script for GCF testing
Hi Guillaume, > Makefile.am |3 ++- > test/hold-and-answer | 20 > 2 files changed, 22 insertions(+), 1 deletions(-) > create mode 100755 test/hold-and-answer patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH_v3 3/3] test: Add hangup-call script for GCF testing
Hi Guillaume,
> Makefile.am |3 ++-
> test/hangup-call | 39 +++
> 2 files changed, 41 insertions(+), 1 deletions(-)
> create mode 100755 test/hangup-call
>
> diff --git a/Makefile.am b/Makefile.am
> index a689483..534245e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -624,7 +624,8 @@ test_scripts = test/backtrace \
> test/swap-calls \
> test/release-and-answer \
> test/hold-and-answer \
> - test/hangup-call-state
> + test/hangup-call-state \
> + test/hangup-call
>
> if TEST
> testdir = $(pkglibdir)/test
> diff --git a/test/hangup-call b/test/hangup-call
> new file mode 100755
> index 000..926efc4
> --- /dev/null
> +++ b/test/hangup-call
> @@ -0,0 +1,39 @@
> +#!/usr/bin/python
> +
> +import sys
> +import dbus
> +
> +bus = dbus.SystemBus()
> +
> +manager = dbus.Interface(bus.get_object('org.ofono', '/'),
> + 'org.ofono.Manager')
> +
> +modems = manager.GetModems()
> +path = modems[0][0]
> +
> +manager = dbus.Interface(bus.get_object('org.ofono', path),
> + 'org.ofono.VoiceCallManager')
> +
> +calls = manager.GetCalls()
> +if (len(calls) == 0):
> + print "No calls available"
> + sys.exit(1)
> +
> +if (len(sys.argv) < 2):
> + i = 0
> + for path, properties in calls:
> + multi = properties["Multiparty"]
> + print "Call ID [ %d ] Multiparty %d" % (i, multi)
> + print
> + i += 1
> +
> + print "Usage: %s [ Call ID ]" % (sys.argv[0])
> + print
> + sys.exit(1)
> +
> +path = calls[int(sys.argv[1])][0]
> +
> +call = dbus.Interface(bus.get_object('org.ofono', path),
> + 'org.ofono.VoiceCall')
> +
> +call.Hangup()
why are we not just asking to provide the object path as argument and
then list-calls can be used to determine which call to hang up. That way
also the hangup-call-state hack is not needed.
Using the object path also makes this less racy since we identify the
call by object path and not a random number that might no longer be
valid.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] test: Tweak hangup script to clear waiting active and held calls
Hi Guillaume, > test/hangup | 12 +++- > 1 files changed, 11 insertions(+), 1 deletions(-) this is a case where you should have done a summary email to describe why are you doing things. > > diff --git a/test/hangup b/test/hangup > index 6444b23..60858a8 100755 > --- a/test/hangup > +++ b/test/hangup > @@ -3,6 +3,16 @@ > import sys > import dbus > > +if (len(sys.argv) < 2): > + print "Usage: %s " % (sys.argv[0]) > + sys.exit(1) > + > +cstate = sys.argv[1] > + > +if cstate != "active" and cstate != "waiting" and cstate != "held": > + print "Valid is active / waiting / held" > + sys.exit(1) > + If you wanna do things like this, then at least the call without parameter should list the current calls and in what state they are. Otherwise this is making things worse. And don't bother renaming the script first. Just introduce a new script and delete the duplicate later on. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/2] test: Rename script hangup-active to hangup
Hi Guillaume, > Makefile.am|2 +- > test/hangup| 29 + > test/hangup-active | 29 - > 3 files changed, 30 insertions(+), 30 deletions(-) > create mode 100755 test/hangup > delete mode 100755 test/hangup-active and why are we renaming this? Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] util: Fix GSM to UTF8 conversion mechanism
Hi Guillaume, > + /* > + * According to the 3GPP specifications 23.038 > + * section 6.2.1.1: > + * In the case there is no character in the extension > + * table, the character of the main default alphabet > + * table or the character from the National Language > + * Locking Shift Table should be displayed. > + */ > >>> > >>> What version of 23.038 are you using? I cannot find such wording in R9 > >>> or R10 specs. > >> > >> It is not the exact wording, I have just rephrased the comment. > >> The exact wording it just below the GSM 7 bits default alphabet > >> extension table in section 6.2.1.1 > >> Generally, do you prefer having an exact extract of the 3GPP spec or > >> just an explanation of what is happening? > >> > > > > The preference is to quote the spec verbatim and enclose the quote in > > "". You can quote fragments of a sentence, etc. > > > > You can paraphrase, but I prefer that we only do that to distill > > something simple, e.g. maximum size of a structure/member. > > > > Ok, I keep his in mind for the next patches. and can someone please run the unit tests if we are touching this: /testutil/Invalid Conversions: ** ERROR:unit/test-util.c:362:test_invalid: assertion failed: (res == NULL) Aborted (core dumped) Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: SIM Insertion Notification
Hi Brian,
> > > I'm running ofono 1.7, with a Cinterion PH8-P plugin I wrote and will
> > submit once I've fully tested it. One problem I have is I never receive
> > notification of SIM insertion, i.e. the Present propertyChanged. Looking
> > at the code it seems pretty obvious why, unless I'm (probably) missing
> > something.
> > >
> > > All the plugins, including mine, do similar to the following:
> > >
> > > static void ph8_pre_sim(struct ofono_modem *modem)
> > > {
> > > struct ph8_data *data = ofono_modem_get_data(modem);
> > > struct ofono_sim *sim;
> > >
> > > ofono_devinfo_create(modem, 0, "atmodem", data->app);
> > > sim = ofono_sim_create(modem, OFONO_VENDOR_CINTERION_PH8,
> > > "atmodem", data->app);
> > >
> > > if (sim&& data->have_sim == TRUE)
> > > ofono_sim_inserted_notify(sim, TRUE);
> > > }
> > >
> > > The call to ofono_sim_create() probes the atmodem driver, which ends
> > with:
> > >
> > > g_idle_add(at_sim_register, sim);
> > >
> > > So the subsequent call to ofono_sim_inserted_notify() will fail, as
> > at_sim_register() is called later when it is scheduled.
> > >
> > > The logs bear this out, so I'm at a loss to understand how anyone
> > receives a SIM notification... What am I missing here?
> >
> > Everything is working as it should, what exactly is the problem? You
> > can call ofono_sim_inserted_notify before the atom is registered, but
> > that means the Present property will be 'True' when you perform the
> > initial GetProperties call.
> >
> > This is meant for devices which do not support SIM hot-swap (e.g. most
> > USB devices and many earlier phones where the SIM is behind the battery).
>
> Thanks for the clarification, Denis. That's what started me looking at the
> Present propertyChanged, as I'd thought all property changes would be
> signalled without the need for a GetProperties first.
>
> I'm using ofono-qt, and the only notifications I get are propertyChanged's
> for CardIdentifier, PreferredLanguages, SubscriberIdentity,
> MobileCountryCode and MobileNetworkCode. I never get Present or any of the
> other properties.
>
> Turns out it's a problem with ofono-qt; it only does a GetProperties once
> when instantiating the SimManager, which is most likely before the interface
> is even available. It never does it again, including when the SIM interface
> becomes available or if the SIM validity changes which could indicate a SIM
> swap. I fixed this and all's working fine now, I'll forward my changes to
> the ofono-qt maintainer.
coming to think about this, it is actually an issue with our API at this
moment. When you add a modem, we do send you the current properties with
it, but when just an interface gets added, we don't. We should give you
the current properties in one go as well.
The API is stable, so we can not fix this until the next major version,
but a switch to D-Bus ObjectManager would clearly make this a lot simple
for application developers. Something for oFono 2.x series.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: Write a ofono plugin
Hi Shaheer, > Kindly, please guide me on the steps of writing a ofono plugin. > Please let me know about any references or so. have a look at the existing plugins inside plugins/ or example/ directories. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/2] test-ussd becomes send-ussd
Hi Denis, > > --- > > test/send-ussd | 75 > > > > test/test-ussd | 75 > > > > 2 files changed, 75 insertions(+), 75 deletions(-) > > create mode 100755 test/send-ussd > > delete mode 100755 test/test-ussd > > > > Why don't we just get rid of test-ussd and create a new send-ussd script > that doesn't do any stdin handling. Something along the lines of: > > ss.GetProperties() > > if state == 'idle' > rsp = ss.Initiate() > elif state == 'user-response' > rsp = ss.Respond() > > print response > > And perhaps another ussd related script that just cancels the request. > Would this satisfy your requirements? are the existing test/initiate-ussd and test/cancel-ussd not good enough? Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: New test scripts ...
Hi Jens, > for personal requirements (debugging) I added 3 simple scripts > (attached) to dump the properties of specific modem interfaces. > > Shall I send patches for them or are they useless for the project? > (For the records: I don't mind either) sending patches would be better. Especially since you might wanna have them added to Makefile.am as well. As a note, have you considered creating one script with different command line options? Our script collection is getting kinda big ;) Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 5/6] unit: Add delivery_report to send_req testing
Hi Ronald, > unit/test-mmsutil.c |3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 4/6] mmsutil: Encode delivery-report in send_req msg
Hi Ronald, > src/mmsutil.c |9 - > src/mmsutil.h |5 + > 2 files changed, 13 insertions(+), 1 deletions(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/6] mmsutil: Add delivery report to send_req struct
Hi Ronald, > src/mmsutil.h |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/6] doc: Modify SendMessage D-Bus API
Hi Ronald,
> doc/service-api.txt |3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/doc/service-api.txt b/doc/service-api.txt
> index 36e1161..57e8942 100644
> --- a/doc/service-api.txt
> +++ b/doc/service-api.txt
> @@ -36,7 +36,8 @@ Methods array{object,dict} GetMessages()
> Possible Errors: [service].Error.InvalidArguments
>[service].Error.TransientFailure
>
> - object SendMessage(array{string} recipients, string smil,
> + object SendMessage(array{string} recipients,
> + boolean delivery-report, string smil,
> array{string id, string content-type,
> string filename})
I think we should do it the same as we do for oFono. Make this is global
settings. Since you don't do that on a per message basis anyway.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCHv2 0/2] mmsd: new tasks definition
Hi Ronald, > These patches concern mmsd and update documentation to describe new > functionalities to add to mmsd. > > Ronald Tessier (2): > doc: Describe delivered group in storage doc > doc: Add new D-Bus methods to service interface both patches have been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/3] doc: Describe delivered group in storage doc
Hi Ronald, > doc/storage.txt | 30 ++ > 1 files changed, 30 insertions(+), 0 deletions(-) > > diff --git a/doc/storage.txt b/doc/storage.txt > index 8e76382..0aa6d9d 100644 > --- a/doc/storage.txt > +++ b/doc/storage.txt > @@ -18,6 +18,7 @@ Meta file Example > [info] > read=false > state=notification > +message_id=0123456789ABCDEF I would just shortcut this to "id". Or is the a reason to keep "message_id" for this? > Meta file Keys/Values details > @@ -31,3 +32,32 @@ state: The message local state, possible values can be: > - "received": m-Retrieve.Conf PDU downloaded and successfully > acknowledged. > - "draft": m-Send.Req PDU ready for sending. > - "sent": m-Send.Req PDU successfully sent. > + > +message_id: this is the value provided in the M-Send.conf PDU (assigned by > MMSC > +in response to a M-Send.req message), this entry will only be created upon > +M-Send.conf message reception if the delivery report was requested. > + > +For sent messages, a group [delivery_status] could take place in addition to > +[info] if delivery report is requested. > +In this group, every recipient has a MMS Delivery status value which can be > one > +of the following: > +- "none": no report has been received yet. > +- "expired": recipient did not retrieve the MMS before expiration. > +- "retrieved": MMS successfully retrieved by the recipient. > +- "rejected": recipient rejected the MMS. > +- "deferred": recipient decided to retrieve the MMS at a later time. > +- "indeterminate": cannot determine if the MMS reached its destination. > +- "forwarded": recipient forwarded the MMS without retrieving it first. > +- "unreachable": recipient is not reachable. > + > + > +Example of a sent_message meta file with delivery report requested > +== > + > +[info] > +state=sent > +message_id=0123456789ABCDEF > + > +[delivery_status] > +21345=retrieved > +09876=none How is this suppose to work. I am kinda failing to see what this tries to solve. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/3] TODO: Add new tasks
Hi Ronald, > TODO | 67 > ++ > 1 files changed, 67 insertions(+), 0 deletions(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/3] doc: Add new D-Bus methods to service interface
Hi Ronald,
> doc/service-api.txt | 20
> 1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/doc/service-api.txt b/doc/service-api.txt
> index 36e1161..5c18cad 100644
> --- a/doc/service-api.txt
> +++ b/doc/service-api.txt
> @@ -63,6 +63,26 @@ Methodsarray{object,dict} GetMessages()
> [service].Error.PermanentContentNotAccepted
> [service].Error.PermanentLackOfPrepaid
>
> + [TODO] void DeleteMessages(array{object})
I do not like the [TODO] annotation here. Left that out.
> +
> + Delete an array of message objects that represents
> + the currently stored messages to delete.
> +
> + Possible Errors: [service].Error.InvalidArguments
> +
> + [TODO] void DeleteConversation(string number)
> +
> + Delete the messages that belongs to the conversation
> + defined by the given number.
> +
> + The number parameter contains digits to look for
> + (i.e.: n last digits of the phone number), only messages
> + with a recipient which ends with the given number will
> + be member of the conversation and thus will be deleted.
> +
> + Possible Errors: [service].Error.InvalidArguments
> +
> +
> Signals MessageAdded(object path, dict properties)
>
> Signal that is sent when a new message is added. It
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 01/12] Constify GDBus method tables
Hi Lucas,
> Constify method tables with the following command:
>
> find . -name '*.[ch]' -exec \
> sed -i 's/\(GDBusMethodTable .* =\)/const \1/g' {} \;
> ---
> plugins/hfp_hf.c|2 +-
> plugins/push-notification.c |2 +-
> plugins/smart-messaging.c |2 +-
> src/audio-settings.c|2 +-
> src/call-barring.c |2 +-
> src/call-forwarding.c |2 +-
> src/call-meter.c|2 +-
> src/call-settings.c |2 +-
> src/call-volume.c |2 +-
> src/cbs.c |2 +-
> src/cdma-connman.c |2 +-
> src/cdma-netreg.c |2 +-
> src/cdma-sms.c |2 +-
> src/cdma-voicecall.c|2 +-
> src/ctm.c |2 +-
> src/gnss.c |2 +-
> src/gprs.c |4 ++--
> src/handsfree.c |2 +-
> src/location-reporting.c|2 +-
> src/manager.c |2 +-
> src/message-waiting.c |2 +-
> src/message.c |2 +-
> src/modem.c |2 +-
> src/network.c |4 ++--
> src/phonebook.c |2 +-
> src/radio-settings.c|2 +-
> src/sim.c |2 +-
> src/sms.c |2 +-
> src/stk.c |2 +-
> src/ussd.c |2 +-
> src/voicecall.c |4 ++--
> 31 files changed, 34 insertions(+), 34 deletions(-)
all 12 patches have been applied.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH mmsd 01/12] Constify GDBus tables
Hi Lucas,
> Constify method tables with the following commands:
>
> find . -name '*.[ch]' -exec sed -i \
>'s/\(c\)\(GDBusSignalTable .* =\)/\1 const \2/g' {} \;
>
> find . -name '*.[ch]' -exec sed -i \
>'s/\(c\)\(GDBusMethodTable .* =\)/\1 const \2/g' {} \;
> ---
> plugins/ofono.c |2 +-
> src/service.c | 12 ++--
> 2 files changed, 7 insertions(+), 7 deletions(-)
all 12 patches have been applied.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH] bluez version 4.99 is required.
Hi Marko, > bluez 4.99 introduced key_size to bt_security struct that is used in > ofono, thus the requirement needs to be higher. > > Signed-off-by: Marko Saukko > --- > configure.ac |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) patch has been applied. However keep in mind no Signed-off-by line here. I fixed that up for you now. Next time, it is your job. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 0/2] mmsd: GetConversation
Hi Ronald, > These patches concern mmsd and are related to lastest patches applied. > > Ronald Tessier (2): > test: Add python get-conversation test > TODO: Update TODO to remove GetConversation entry > > Makefile.am |3 ++- > TODO | 10 -- > test/get-conversation | 34 ++ > 3 files changed, 36 insertions(+), 11 deletions(-) > create mode 100755 test/get-conversation both patches have been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: mms: next steps
Hi Ronald, > Now that mms' TODO is almost empty (I've sent a set of patches yesterday > that implement the last entry of the TODO). > What should we add to release a first version of MMS ? > > What's already in place : > - MMS submission, > - MMS reception (automatic retrieval support), > - MMS storage with status support, > - (Basic) error management with transaction completion on restart. > > What can be added : > - Manual retrieval support for MMS reception, I think we should not do that. Manual has never been a good idea. Lets try to just have proper error recovery and do this without user interaction. > - Delivery report support (M-Delivered.ind), Sounds like a good thing to add now. > - Enhance D-Bus API (DeleteMessages(), DeleteConversation() ...) Send proposals for this. Sounds on the first look reasonable. However the devil might be in the details. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCHv2 0/4] mmsd: GetConversation support
Hi Ronald, > These patches concern mmsd and implement "GetConversation" D-Bus API. > > Ronald Tessier (4): > service: Retrieve get_conversation() args > service: Fill conversation list with messages > service: Sort conversation list by message date > doc: Update service-api.txt > > doc/service-api.txt |6 ++ > src/service.c | 141 > +++ > 2 files changed, 147 insertions(+), 0 deletions(-) all 4 patches have been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] doc: Update service-api doc to match the code
Hi Ronald,
> GetMessages method has no argument and the implementation does not
> return any error.
>
> ---
> doc/service-api.txt |2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/doc/service-api.txt b/doc/service-api.txt
> index 46989f4..d87644b 100644
> --- a/doc/service-api.txt
> +++ b/doc/service-api.txt
> @@ -16,8 +16,6 @@ Methods array{object,dict} GetMessages()
> and removal shall be monitored via MessageAdded and
> MessageRemoved signals.
>
> - Possible Errors: [service].Error.InvalidArguments
> -
actually this error is valid. For example in case you call GetMessages
with an argument. So the error should stay.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/1] TODO: task complete
Hi Sebastien, > --- > TODO | 14 -- > 1 files changed, 0 insertions(+), 14 deletions(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 0/2] mmsd: error management
Hi Sebastien, > The first patch removes the MMS reception part from TODO file which > has been done some times ago. > This model of error management applies also for MMS emission, except that > it shouldn't be silent because initiated by the user. > > Sébastien Bianti (2): > TODO: MMS reception error management done > service: emit signal when send has failed > > TODO | 13 - > src/service.c | 32 > 2 files changed, 20 insertions(+), 25 deletions(-) both patches have been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCHv1 1/5] service: Add GetConversation method
Hi Ronald, > src/service.c | 21 + > 1 files changed, 21 insertions(+), 0 deletions(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCHv1 2/5] service: Retrieve get_conversation() args
Hi Ronald,
> ---
> src/service.c | 37 +
> 1 files changed, 37 insertions(+), 0 deletions(-)
>
> diff --git a/src/service.c b/src/service.c
> index 24c89b6..18c11f4 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -690,11 +690,48 @@ static DBusMessage *get_messages(DBusConnection *conn,
> return reply;
> }
>
> +static gboolean get_conversation_get_args(DBusMessage *dbus_msg,
> + const char **number, unsigned int *count)
> +{
> + DBusMessageIter top_iter;
> +
> + if (dbus_message_iter_init(dbus_msg, &top_iter) == FALSE)
> + return FALSE;
> +
> + if (dbus_message_iter_get_arg_type(&top_iter) != DBUS_TYPE_STRING)
> + return FALSE;
> +
> + dbus_message_iter_get_basic(&top_iter, number);
> + if (*number[0] == '\0')
> + return FALSE;
> +
> + if (valid_number_format(*number) == FALSE)
> + return FALSE;
> +
> + if (!dbus_message_iter_next(&top_iter))
> + return FALSE;
> +
> + if (dbus_message_iter_get_arg_type(&top_iter) != DBUS_TYPE_UINT32)
> + return FALSE;
> +
> + dbus_message_iter_get_basic(&top_iter, count);
> +
> + return TRUE;
> +}
> +
> static DBusMessage *get_conversation(DBusConnection *conn,
> DBusMessage *dbus_msg, void *data)
> {
> DBusMessage *reply;
> DBusMessageIter iter, array;
> + const char *number;
> + unsigned int count;
> +
> + if (get_conversation_get_args(dbus_msg, &number, &count) == FALSE) {
> + mms_debug("Invalid arguments");
> +
> + return __mms_error_invalid_args(dbus_msg);
> + }
in this case I don't really see that it is useful to split this into a
separate function. And the debug print is actually useless. Please
remove that one.
>
> reply = dbus_message_new_method_return(dbus_msg);
> if (reply == NULL)
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCHv1 3/5] service: Fill conversation list with messages
Hi Ronald,
> ---
> src/service.c | 88
> -
> 1 files changed, 87 insertions(+), 1 deletions(-)
>
> diff --git a/src/service.c b/src/service.c
> index 18c11f4..2787c3a 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -719,13 +719,85 @@ static gboolean get_conversation_get_args(DBusMessage
> *dbus_msg,
> return TRUE;
> }
>
> +static gboolean is_recipient(const char *recipients, const char *number)
> +{
> + const char *rec, *num;
> +
> + while (*recipients != '\0') {
> + rec = recipients;
> + num = number;
> +
> + while (*rec != '\0' && *num != '\0') {
> + if (*rec == *num) {
> + rec++;
> + num++;
> + } else {
> + if (*rec == '-' || *rec == '.') {
> + rec++;
> + continue;
> + }
> +
> + if (*num == '-' || *num == '.') {
> + num++;
> + continue;
> + }
> +
> + rec++;
> + break;
> + }
> + }
> +
> + /*
> + * Phone numbers in recipients end with /TYPE=PLMN, so the
> + * wanted number is found if the whole string number is found
> + * and if the matched phone number ends with the wanted number
> + * (i.e.: "/TYPE=PLMN" must follow).
> + */
> + if (*num == '\0' && *rec == '/')
> + return TRUE;
> +
> + recipients = rec;
> + }
> +
> + return FALSE;
> +}
please explain what this function is actually doing? Adding a comment
seems like a good idea here.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCHv1 4/5] service: Sort conversation list by message date
Hi Ronald,
> src/service.c | 27 +++
> 1 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/src/service.c b/src/service.c
> index 2787c3a..1bd4671 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -719,6 +719,31 @@ static gboolean get_conversation_get_args(DBusMessage
> *dbus_msg,
> return TRUE;
> }
>
> +static gint fill_conversation_sort(gconstpointer a, gconstpointer b)
> +{
> + const struct mms_message *msg1 = a;
> + const struct mms_message *msg2 = b;
> + time_t date1, date2;
> +
> + if (msg1->type == MMS_MESSAGE_TYPE_SEND_REQ)
> + date1 = msg1->sr.date;
> + else
> + date1 = msg1->rc.date;
> +
> + if (msg2->type == MMS_MESSAGE_TYPE_SEND_REQ)
> + date2 = msg2->sr.date;
> + else
> + date2 = msg2->rc.date;
> +
> + if (date1 > date2)
> + return 1;
> +
> + if (date1 < date2)
> + return -1;
> +
> + return 0;
> +}
> +
> static gboolean is_recipient(const char *recipients, const char *number)
> {
> const char *rec, *num;
> @@ -784,6 +809,8 @@ static GList *fill_conversation(const struct mms_service
> *service,
> conversation = g_list_prepend(conversation, value);
> }
>
> + conversation = g_list_sort(conversation, fill_conversation_sort);
> +
is it really more efficient to prepend and then later sort the list? Can
we just not insert items sorted to begin with?
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCHv1 5/5] doc: Update service-api.txt
Hi Ronald,
> doc/service-api.txt |9 -
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/doc/service-api.txt b/doc/service-api.txt
> index 46989f4..99e80be 100644
> --- a/doc/service-api.txt
> +++ b/doc/service-api.txt
> @@ -24,11 +24,18 @@ Methods array{object,dict} GetMessages()
> that are part of a conversation between the service
> entity and the number provided.
>
> + The number parameter must only contains significant
> digits
actually using "must" is a wrong description.
> + to look for (i.e.: n last digits of the phone number),
> only
> + messages with a recipient containing the given number
> will
> + be part of the GetConversation result.
And I am not sure we do wanna make some sort of assumption here anyway.
These are not CLIP numbers where the operators don't seem to get this
right. For SMS and MMS is seems the international numbering plan is
used.
Or do you have counter examples. If so, please post them here first.
> +
> The count parameter can either be 0 for unlimited
> messages in the conversation or limit the conversation
> to count last messages.
>
> - Possible Errors: [service].Error.InvalidArguments
> + Possible Errors:
> + [service].Error.InvalidArguments
> + [service].Error.TransientFailure
Please don't invent your own indentation here. Keep it aligned as all
the other documents in oFono and mmsd do.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH v3 0/2] mmsd: bug fix
Hi Sebastien, > mmsutil: add mms_message_status_get_string API > service: use proper status value string > > src/mmsutil.c | 18 ++ > src/mmsutil.h |1 + > src/service.c |2 +- > 3 files changed, 20 insertions(+), 1 deletions(-) both patches have been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/3] service: fix for message_delete bug
Hi Sebastien, > mms_message_unregister destroyes the message > --- > src/service.c |7 ++- > 1 files changed, 6 insertions(+), 1 deletions(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/3] mmsutil: add mms_message_status_get_string API
Hi Sebastien,
> src/mmsutil.c | 25 +
> src/mmsutil.h |1 +
> 2 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/src/mmsutil.c b/src/mmsutil.c
> index c507d6f..725849b 100644
> --- a/src/mmsutil.c
> +++ b/src/mmsutil.c
> @@ -1661,3 +1661,28 @@ gboolean mms_message_encode(struct mms_message *msg,
> int fd)
>
> return FALSE;
> }
> +
> +char *mms_message_status_get_string(enum mms_message_status status)
> +{
> + char *status_str;
> +
> + switch (status) {
> + case MMS_MESSAGE_STATUS_DOWNLOADED:
> + status_str = "downloaded";
> + break;
> + case MMS_MESSAGE_STATUS_RECEIVED:
> + status_str = "received";
> + break;
> + case MMS_MESSAGE_STATUS_READ:
> + status_str = "read";
> + break;
> + case MMS_MESSAGE_STATUS_SENT:
> + status_str = "sent";
> + break;
> + case MMS_MESSAGE_STATUS_DRAFT:
> + status_str = "draft";
> + break;
> + }
> +
> + return status_str;
> +}
why bother with status_str here. Just call return right away. Also the
return value is const char *.
> diff --git a/src/mmsutil.h b/src/mmsutil.h
> index b2a0418..d3b507f 100644
> --- a/src/mmsutil.h
> +++ b/src/mmsutil.h
> @@ -146,3 +146,4 @@ gboolean mms_message_decode(const unsigned char *pdu,
> unsigned int len, struct mms_message *out);
> gboolean mms_message_encode(struct mms_message *msg, int fd);
> void mms_message_free(struct mms_message *msg);
> +char *mms_message_status_get_string(enum mms_message_status status);
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCHv3 00/10] mmsd: error management
Hi Ronald, > These patches concern mmsd and are related to error handling while > receiving message. > The main idea is to try to recover error internally when possible > and drop messages that are not decodable. > > Please, ignore previous set of patches sent by Sebastien (when he > reworked my patches) since these are the same except the first > patch (don't always remove message's files, only when the message > is not decodable). > > > Ronald Tessier (1): > service: remove files when unable to decode received msg > > Sébastien Bianti (9): > service: add debug prints > service: remove useless variable > service: add attempts counter variable > service: move destroy request into process_request > service: refactoring file closure > service: callbacks should return a boolean > service: refactoring error code > service: add requeue functionality > service: bearer activity needs to be checked > > src/service.c | 136 ++-- > 1 files changed, 92 insertions(+), 44 deletions(-) all 10 patches have been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH_v2] atmodem: Poll SIM state after having entered PIN
Hi Guillaume,
> Some modems use SIM notification to check SIM state.
> If not do some poll using atutil helper.
> ---
> drivers/atmodem/sim.c | 28
> 1 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
> index c51b1d4..269079a 100644
> --- a/drivers/atmodem/sim.c
> +++ b/drivers/atmodem/sim.c
> @@ -51,6 +51,7 @@ struct sim_data {
> GAtChat *chat;
> unsigned int vendor;
> guint ready_id;
> + struct at_util_sim_state_query *sim_state_query;
> };
>
> static const char *crsm_prefix[] = { "+CRSM:", NULL };
> @@ -972,6 +973,23 @@ static void at_epev_notify(GAtResult *result, gpointer
> user_data)
> sd->ready_id = 0;
> }
>
> +static void sim_state_cb(gboolean present, gpointer user_data)
> +{
> + struct cb_data *cbd = user_data;
> + struct sim_data *sd = cbd->user;
> + ofono_sim_lock_unlock_cb_t cb = cbd->cb;
> +
> + at_util_sim_state_query_free(sd->sim_state_query);
> + sd->sim_state_query = NULL;
> +
> + if (present == 1)
> + CALLBACK_WITH_SUCCESS(cb, cbd->data);
> + else
> + CALLBACK_WITH_FAILURE(cb, cbd->data);
> +
> + g_free(cbd);
> +}
> +
> static void at_pin_send_cb(gboolean ok, GAtResult *result,
> gpointer user_data)
> {
> @@ -1005,6 +1023,13 @@ static void at_pin_send_cb(gboolean ok, GAtResult
> *result,
> sd->ready_id = g_at_chat_register(sd->chat, "*EPEV",
> at_epev_notify,
> FALSE, cbd, g_free);
> + /*
> + * After pin is entered, SIM state is check by doing
> + * some polling if modem doesn't use notification.
> + */
> + default:
> + sd->sim_state_query = at_util_sim_state_query_new(sd->chat,
> + 2, 20, sim_state_cb, cbd);
> return;
> }
this part just looks wrong. You are now also doing CPIN polling for the
Ericsson and ST-Ericsson cards.
>
> @@ -1246,6 +1271,9 @@ static void at_sim_remove(struct ofono_sim *sim)
> {
> struct sim_data *sd = ofono_sim_get_data(sim);
>
> + /* Cleanup potential SIM state polling */
> + at_util_sim_state_query_free(sd->sim_state_query);
> +
Who owns the cbd data in this case? Are we not leaking that here?
> ofono_sim_set_data(sim, NULL);
>
> g_at_chat_unref(sd->chat);
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/3] Speedup: Use speedup specific driver for ussd
Hi Nicolas, > plugins/speedup.c |6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/plugins/speedup.c b/plugins/speedup.c > index ca6ed13..8b95286 100644 > --- a/plugins/speedup.c > +++ b/plugins/speedup.c > @@ -2,7 +2,7 @@ > * > * oFono - Open Source Telephony > * > - * Copyright (C) 2008-2011 Intel Corporation. All rights reserved. > + * Copyright (C) 2008-2012 Intel Corporation. All rights reserved. > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2 as > @@ -282,8 +282,8 @@ static void speedup_post_online(struct ofono_modem *modem) > > ofono_cbs_create(modem, OFONO_VENDOR_QUALCOMM_MSM, > "atmodem", data->aux); > - ofono_ussd_create(modem, OFONO_VENDOR_QUALCOMM_MSM, > - "atmodem", data->aux); > + ofono_ussd_create(modem, OFONO_VENDOR_SPEEDUP, > + "speedupmodem", data->aux); you do not need both here. Either "speedupmodem" or OFONO_VENDOR_SPEEDUP quirk. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH v1 2/2] service: handle error while receiving messages
Hi Ronald,
> src/service.c | 94
> 1 files changed, 74 insertions(+), 20 deletions(-)
>
> diff --git a/src/service.c b/src/service.c
> index 851db34..c3fc0c6 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -55,6 +55,7 @@
> #define CONTENT_TYPE_APP_SMIL "Content-Type:
> \"application/smil\";charset=utf-8"
>
> #define MAX_ATTACHMENTS_NUMBER 25
> +#define MAX_ATTEMPTS 3
>
> #define uninitialized_var(x) x = x
>
> @@ -98,6 +99,7 @@ struct mms_request {
> gsize data_size;
> int fd;
> guint16 status;
> + guint16 attempt;
> struct mms_service *service;
> mms_request_result_cb_t result_cb;
> struct mms_message *msg;
> @@ -526,6 +528,8 @@ static struct mms_request *create_request(enum
> mms_request_type type,
>
> request->status = 0;
>
> + request->attempt = 0;
> +
> return request;
> }
>
> @@ -1656,17 +1660,22 @@ int mms_service_set_bearer_handler(struct mms_service
> *service,
> return 0;
> }
>
> -static void deactivate_bearer(struct mms_service *service)
> +static inline gboolean bearer_is_active(struct mms_service *service)
> {
> - DBG("service %p", service);
> -
> if (service->bearer_setup == TRUE)
> - return;
> -
> - if (service->bearer_active == FALSE)
> - return;
> + return FALSE;
>
> if (service->bearer_handler == NULL)
> + return FALSE;
> +
> + return service->bearer_active;
> +}
> +
> +static void deactivate_bearer(struct mms_service *service)
> +{
> + DBG("service %p", service);
> +
> + if (bearer_is_active(service) == FALSE)
> return;
>
> DBG("service %p", service);
> @@ -1798,16 +1807,42 @@ exit:
> munmap(pdu, len);
> }
>
> +static gboolean mms_requeue_request(struct mms_request *request)
> +{
> + request->attempt += 1;
> +
> + if (request->attempt == MAX_ATTEMPTS)
> + return FALSE;
> +
> + if (request->type == MMS_REQUEST_TYPE_GET) {
> + request->fd = open(request->data_path, O_WRONLY | O_TRUNC,
> + S_IWUSR | S_IRUSR);
> + if (request->fd < 0)
> + return FALSE;
> + }
> +
> + g_queue_push_tail(request->service->request_queue, request);
> +
> + return TRUE;
> +}
> +
> static gboolean web_get_cb(GWebResult *result, gpointer user_data)
> {
> gsize written;
> gsize chunk_size;
> struct mms_request *request = user_data;
> - struct mms_service *service;
> const guint8 *chunk;
>
> - if (g_web_result_get_chunk(result, &chunk, &chunk_size) == FALSE)
> - goto error;
> + if (g_web_result_get_chunk(result, &chunk, &chunk_size) == FALSE) {
> + mms_error("Fail to get data chunk");
> +
> + close(request->fd);
> +
> + if (mms_requeue_request(request) == FALSE)
> + unlink(request->data_path);
> +
> + goto exit;
> + }
>
> if (chunk_size == 0) {
> close(request->fd);
> @@ -1826,26 +1861,35 @@ static gboolean web_get_cb(GWebResult *result,
> gpointer user_data)
>
> if (written != chunk_size) {
> mms_error("only %zd/%zd bytes written\n", written, chunk_size);
> - goto error;
> +
> + close(request->fd);
> + unlink(request->data_path);
> +
> + goto complete;
> }
>
same here. I do not like this. Please make clean and small changes that
are not re-factoring major pieces. If re-factoring seems to be needed,
please separate them out so they can be reviewed.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
