Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Re: [PATCH] p2p technology disable will stop P2P find
(Daniel Wagner)
2. Re: Connman + ofono + telit le910v2 : no service (Daniel Wagner)
3. Re: What is the best way to refresh connections ? (Daniel Wagner)
4. Re: Connman + ofono + telit le910v2 : no service (Louis Rannou)
5. [PATCH 0/2] stop scan on p2p technology disable (Vasyl Vavrychuk)
----------------------------------------------------------------------
Message: 1
Date: Thu, 7 Dec 2017 21:38:38 +0100
From: Daniel Wagner <[email protected]>
To: Vasyl Vavrychuk <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] p2p technology disable will stop P2P find
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Vasyl,
On 12/01/2017 06:47 PM, Vasyl Vavrychuk wrote:
> patch attached
inline patches are preferred, e.g. via git send-email
> 947d93483e8b62d9afd47c3cb4a6c614f6cc3941.patch
>
>
> From 947d93483e8b62d9afd47c3cb4a6c614f6cc3941 Mon Sep 17 00:00:00 2001
> From: Vasyl Vavrychuk<[email protected]>
> Date: Thu, 30 Nov 2017 14:14:32 +0200
> Subject: [PATCH] p2p technology disable will stop P2P find
The subject should have a prefix. Since you touch several files in this
patch it is not so simple to chose the right one. Though after a quick
glance I think this patch should be splitted into two anyway. Then it
get's ease :)
>
> ---
> include/device.h | 4 +++-
> plugins/wifi.c | 19 +++++++++++++++++++
> src/device.c | 49 +++++++++++++++++++++++++++++++++++++++++--------
> src/technology.c | 1 +
> 4 files changed, 64 insertions(+), 9 deletions(-)
>
> diff --git a/include/device.h b/include/device.h
> index 9ac800a..0eb3ce5 100644
> --- a/include/device.h
> +++ b/include/device.h
> @@ -119,11 +119,13 @@ struct connman_device_driver {
> void (*remove) (struct connman_device *device);
> int (*enable) (struct connman_device *device);
> int (*disable) (struct connman_device *device);
> - int (*scan)(enum connman_service_type type,
> + int (*scan) (enum connman_service_type type,
It's better not to change the whitespace here. It introduces just churn
in the git history without a real benefit.
> struct connman_device *device,
> const char *ssid, unsigned int ssid_len,
> const char*identity, const char* passphrase,
> const char *security, void *user_data);
> + void (*stop_scan) (enum connman_service_type type,
> + struct connman_device *device);
Make one patch which introduces this new interface.
> int (*set_regdom) (struct connman_device *device,
> const char *alpha2);
> };
> diff --git a/plugins/wifi.c b/plugins/wifi.c
> index 6efaad6..a842999 100644
> --- a/plugins/wifi.c
> +++ b/plugins/wifi.c
> @@ -1913,6 +1913,24 @@ static int wifi_scan(enum connman_service_type type,
> return ret;
> }
>
> +static void wifi_stop_scan(enum connman_service_type type,
> + struct connman_device *device)
> +{
> + struct wifi_data *wifi = connman_device_get_data(device);
> +
> + DBG("device %p wifi %p", device, wifi);
> +
> + if (!wifi)
> + return;
> +
> + if (type == CONNMAN_SERVICE_TYPE_P2P) {
> + if (wifi->p2p_find_timeout) {
> + g_source_remove(wifi->p2p_find_timeout);
> + p2p_find_stop(device);
> + }
> + }
> +}
> +
> static void wifi_regdom_callback(int result,
> const char *alpha2,
> void *user_data)
> @@ -1952,6 +1970,7 @@ static struct connman_device_driver wifi_ng_driver = {
> .enable = wifi_enable,
> .disable = wifi_disable,
> .scan = wifi_scan,
> + .stop_scan = wifi_stop_scan,
> .set_regdom = wifi_set_regdom,
> };
And the above two chunks are the second patch with the implementation.
>
> diff --git a/src/device.c b/src/device.c
> index a563f46..d880e64 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -146,12 +146,32 @@ enum connman_service_type
> __connman_device_get_service_type(
> return CONNMAN_SERVICE_TYPE_CELLULAR;
> case CONNMAN_DEVICE_TYPE_GADGET:
> return CONNMAN_SERVICE_TYPE_GADGET;
> -
> }
No whitespace changes here please.
>
> return CONNMAN_SERVICE_TYPE_UNKNOWN;
> }
>
> +/**
> + * Find out if device is reponsible for supporting specific service_type.
> + */
The comment doesn't really give additional information. The function
name has all in it. So either add real information into the comment or
just leave it away.
> +static bool __connman_device_has_service_type(
Since it is a local function you don't have to prefix with __connman.
> + struct connman_device *device, enum
> connman_service_type service_type)
In ordedr to keep it slightly more consistent use type instead
service_type. There is already one function in this file which function
signature is
static int wifi_scan(enum connman_service_type type,
But then I realize later in this file we also have service_type. Well,
still I prefer shorter variable names :)
> +{
> + enum connman_service_type device_service_type =
> + __connman_device_get_service_type(device);
> +
The you make the name device_service_type a bit shorter.
> + if ((service_type == CONNMAN_SERVICE_TYPE_UNKNOWN) ||
> + (device_service_type == CONNMAN_SERVICE_TYPE_UNKNOWN))
> + return false;
The parentheses are not necessary. C programmers should know the c
operator precedence ;)
> +
> + if (device_service_type == CONNMAN_SERVICE_TYPE_WIFI) {
> + return (service_type == CONNMAN_SERVICE_TYPE_WIFI) ||
> + (service_type == CONNMAN_SERVICE_TYPE_P2P);
> + } else {
> + return service_type == device_service_type;
> + }
> +}
> +
> static gboolean device_pending_reset(gpointer user_data)
> {
> struct connman_device *device = user_data;
> @@ -1069,13 +1089,9 @@ int __connman_device_request_scan(enum
> connman_service_type type)
> enum connman_service_type service_type =
> __connman_device_get_service_type(device);
>
> - if (service_type != CONNMAN_SERVICE_TYPE_UNKNOWN) {
> - if (type == CONNMAN_SERVICE_TYPE_P2P) {
> - if (service_type != CONNMAN_SERVICE_TYPE_WIFI)
> - continue;
> - } else if (service_type != type)
> - continue;
> - }
> + if ((service_type != CONNMAN_SERVICE_TYPE_UNKNOWN) &&
> + !__connman_device_has_service_type(device, type))
> + continue;
>
> err = device_scan(type, device);
> if (err == 0 || err == -EALREADY || err == -EINPROGRESS) {
> @@ -1108,6 +1124,23 @@ int __connman_device_request_hidden_scan(struct
> connman_device *device,
> passphrase, security, user_data);
> }
>
> +void __connman_device_stop_scan(enum connman_service_type type)
> +{
> + GSList *list;
New line after the declaration please.
> + for (list = device_list; list; list = list->next) {
> + struct connman_device *device = list->data;
> + enum connman_service_type service_type =
> + __connman_device_get_service_type(device);
> +
> + if ((service_type != CONNMAN_SERVICE_TYPE_UNKNOWN) &&
> + !__connman_device_has_service_type(device, type))
Here and above you test first if service_type != UNKNOWN. In
__connman_device_has_service_type you have another test if the type is
UNKNOWN. I suggest you leave the in the function and get rid of the test
here and inte function above.
> + continue;
> +
> + if (device->driver && device->driver->stop_scan)
> + device->driver->stop_scan(type, device);
> + }
> +}
> +
> static char *index2ident(int index, const char *prefix)
> {
> struct ifreq ifr;
> diff --git a/src/technology.c b/src/technology.c
> index d2f0ae2..3df051e 100644
> --- a/src/technology.c
> +++ b/src/technology.c
> @@ -787,6 +787,7 @@ static int technology_disable(struct connman_technology
> *technology)
>
> if (technology->type == CONNMAN_SERVICE_TYPE_P2P) {
> technology->enable_persistent = false;
> + __connman_device_stop_scan(CONNMAN_SERVICE_TYPE_P2P);
> return technology_disabled(technology);
> } else if (technology->type == CONNMAN_SERVICE_TYPE_WIFI) {
> struct connman_technology *p2p;
> --
> libgit2 0.26.0
Thanks,
Daniel
------------------------------
Message: 2
Date: Thu, 7 Dec 2017 21:47:12 +0100
From: Daniel Wagner <[email protected]>
To: Louis Rannou <[email protected]>, [email protected]
Subject: Re: Connman + ofono + telit le910v2 : no service
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
On 12/04/2017 03:47 PM, Louis Rannou wrote:
> Hello,
>
> On 01/12/2017 15:48, Christophe Ronco wrote:
>> Hi Louis,
>>
>>
>> On 12/01/2017 03:27 PM, Louis Rannou wrote:
>>> Hello,
>>>
>>> I am trying to get a connection with a GSM modem Telit LE910 v2.
>>> When I power it on, everything goes well, the technology is present :
>>> connmanctl> technologies
>>> /net/connman/technology/cellular
>>> ?? Name = Cellular
>>> ?? Type = cellular
>>> ?? Powered = True
>>> ?? Connected = False
>>> ?? Tethering = False
>>> /net/connman/technology/ethernet
>>> ?? Name = Wired
>>> ?? Type = ethernet
>>> ?? Powered = True
>>> ?? Connected = True
>>> ?? Tethering = False
>>>
>>> ?but I don't get the associated service (only the service from the
>>> ethernet connection) :
>>> connmanctl> services
>>> *AR Wired??????????????? ethernet_f8811a0424e0_cable
>>>
>>> I can force the activation through dbus, but that's not very satisfying.
>>>
>>> dbus-send --system --print-reply --dest=org.ofono /telit_0/context1
>>> org.ofono.ConnectionContext.SetProperty string:"Active"
>>> variant:boolean:"true"
>>>
>>> Files attached are logs from connman and ofono merged together, and
>>> the d-bus capture.
>>>
>>> Am I missing something ?
>> I don't know this modem very well, but I had a quick look at the logs
>> and I think your APN is empty.
>> Try to provision a non empty APN for your operator (F-Bouygues
>> Telecom, 208/20) in ofono.
>> If I remember well, connman creates a service only if APN is not empty.
>>
>> Christophe Ronco
>> _______________________________________________
>> connman mailing list
>> [email protected]
>> https://lists.01.org/mailman/listinfo/connman
>
> Thanks for the reply,
>
> Doing so creates the service (cellular_208201401625679_context1) but
> does not activate it. If I force the activation through connmanctl,
> connman creates a new ethernet service and links it to my network
> interface where a dhcp will give a wrong address.
Not sure if the 'wrong address' is pointing to the bug report we had:
https://lists.01.org/pipermail/connman/2017-September/022092.html
> Finally I need to configure the APN, force the activation and force the
> address, the route and the arp.
>
> Maybe there is any better way to do it ?
If the hardware is not completely broken you should be able to use it
without any additinal quirks from your side. When you enable the
'technology' via the D-Bus APIs of ConnMan, the modem should be powered
up and ConnMan should then present you a new Service. And when you tell
ConnMan to connect to this service the routing etc should be setup
correctly.
So something is clearly not working correctly :(
Thanks,
Daniel
------------------------------
Message: 3
Date: Thu, 7 Dec 2017 22:05:24 +0100
From: Daniel Wagner <[email protected]>
To: Louis Rannou <[email protected]>
Cc: [email protected]
Subject: Re: What is the best way to refresh connections ?
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Louis,
On 12/06/2017 09:34 AM, Louis Rannou wrote:
> Hello,
>
> I have several network services connected. My goal is to check if the
> main service is connected to the internet (able to access a portal), and
> if not, switch to one connected (if there is one).
The first service will always check if there is internet connectivity
(ONLINE vs READY state).
The online check has a several limitation. For example it is only
executed once. There are no rechecks after that. Changing this is not a
simple task, because we can't really say when we can do the online test
and when it is okay to fail. Think about IPv4 and IPv6 and when we send
out two online checks one for IPv4 and one for IPv6. The first fails and
the second takes very long or the packet got just lost.
So from our past experiences with the online check is that before you
can add something here we need to have a lot of test infrastructure in
place to test the state machine. Currently our state machine is very
complex and has a lot of special corner cases implemented.
I have started to write a small end to end test framework but never got
to a point where it is useful. I might find some time over the coming
weeks to do some real work on it. As long we don't have testing for our
state machine I fear adding another layer of complexity will be hard to
justify.
> Therefore I want to implement a new service API (ex: refresh). I may
> have two options :
>
> *1/* check the main service's connection and downgrade its state to
> READY. Then connman should check every service to find one online (only
> with option EnableOnlineCheck).*
> 2/* find the first service that has a connection and move it before the
> main connection.
>
> In my opinion, the second option is easier to implement as all
> mechanisms needed already exist.
You can try this and see how good it works. That should indeed work more
or less.
> To check the connection, I was thinking about using the function
> /wispr_portal_request_portal(*wp_context)/.
There is a patch from the mer project which does continuously online
checks.
> I am not really aware about connman's logic yet, so I'm open to your
> suggestions.
The state machine is quite complex these days. It used to be much
simpler but people like to add features. That is why no one really
understands it and that is also the reason why I am hesitating adding
new stuff without end to end tests.
Thanks,
Daniel
------------------------------
Message: 4
Date: Fri, 8 Dec 2017 12:05:14 +0100
From: Louis Rannou <[email protected]>
To: Daniel Wagner <[email protected]>, [email protected]
Subject: Re: Connman + ofono + telit le910v2 : no service
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"
On 07/12/2017 21:47, Daniel Wagner wrote:
> Not sure if the 'wrong address' is pointing to the bug report we had:
>
> https://lists.01.org/pipermail/connman/2017-September/022092.html
It's probably a bit different. Ofono must be fine as the
ConnectionContext properties are suggesting a correct network
configuration (address, netmask, gateway). But the Telit needs the
network interface (usb0 on my case) to configured with the sames
settings (see ofono /doc/telit-modem.txt).
I think I need a semi-automatic network configuration. It's not dhcp
neither it is manual. It needs to get the address, netmask and gateway
from ofono using the ConnectionContext API. I guess I need some dev in
connman for this special configuration.
>> Finally I need to configure the APN, force the activation and force
>> the address, the route and the arp.
>>
>> Maybe there is any better way to do it ?
>
> If the hardware is not completely broken you should be able to use it
> without any additinal quirks from your side. When you enable the
> 'technology' via the D-Bus APIs of ConnMan, the modem should be
> powered up and ConnMan should then present you a new Service. And when
> you tell ConnMan to connect to this service the routing etc should be
> setup correctly.
>
> So something is clearly not working correctly :(
Ok, I need to investigate on this.
>
> Thanks,
> Daniel
Thank you,
Louis.
--
Logo <http://www.smile.eu/>
14 rue G?nissieu
38000 Grenoble
*Louis RANNOU*
Ing?nieur ?tudes et D?veloppement
email [email protected] <mailto:[email protected]>
url http://www.smile.eu
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.01.org/pipermail/connman/attachments/20171208/445d0f6f/attachment-0001.html>
------------------------------
Message: 5
Date: Fri, 8 Dec 2017 16:19:05 +0200
From: Vasyl Vavrychuk <[email protected]>
To: [email protected]
Cc: Vasyl Vavrychuk <[email protected]>
Subject: [PATCH 0/2] stop scan on p2p technology disable
Message-ID: <[email protected]>
Vasyl Vavrychuk (2):
device: add stop_scan method to device interface
wifi: stop scan on p2p technology disable
include/device.h | 2 ++
plugins/wifi.c | 19 +++++++++++++++++++
src/connman.h | 1 +
src/device.c | 47 ++++++++++++++++++++++++++++++++++++++---------
src/technology.c | 1 +
5 files changed, 61 insertions(+), 9 deletions(-)
--
2.11.0
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 26, Issue 7
**************************************
