Hi Tomasz,

On 27.01.2012 13:55, Tomasz Bursztyka wrote:
> Hi,
> 
> Here is a patchset that fixes and introduces new properties/behaviors into 
> the Session API.
> 
> First change is the move from Online property to a State property which shows 
> different states: disconnected, connected or online.
> This maps the service's state: ready -> connected, online -> online, all 
> other: disconnected.
> 
> Due to that, I propose in patch 2, 3, 4, 5, 6 several changes in 
> notifications:
> - session does not notify anything about Name/Bearer/Interface/Ipv4/Ipv6 
> until it reaches a relevant state: connected and/or online.

We should clear this list when we go offline as well.

> It is imho, useless to notify those informations when state is still 
> disconnected. Since they rely on underlying service, we notifify 
> only when this service gets connected.

My UI shows me bogus information when the state is changed to
disconnect, e.g. IPv4 etc which is not valid anymore.

> - the patch 3 refactor the code to decide "when" is it needed to notify 
> changes. This simplifies and adds more flexibility in taking decision 
> on relevant notification while centralizing this decision in one unique place.

Looks good.

> - patch 4 follows patch 2, we don't advertize the ipconfig changes if the 
> session is not yet in a relevant state.
> - patch 5 and 6 are a bit different, to save "space" in notified content: it 
> does not provide ipconfig  (ipv4 and/or ipv6) info if its info are not 
> relevant ( = in a connected state). This follows actually what service API 
> does, when requesting its settings. It just pushes the idea a bit more: we 
> don't 
> add a empty Ipv4/6 key in the dict if not relevant.

I noticed that there a quite a few smaller things which do not work e.g.

A session is online, the user pressed Session.Connect(). Then the
AllowedBearer is changed to new bearer (e.g. ethernet -> wifi), the
first one will stay online and the new one offline. StayConnected is of
no use.

> Next comes the introduction of a new property, giving the possibility to 
> "filter" out which connection we want from the session: local (we don't care 
> to be
> connected to internet), internet (we absolutely want that) or any: we want to 
> get local or internet connection (session's state will change as usual and we 
> will be notified every time).
> One question however: in local type, it means that if underlying service gets 
> "online", session won't be "connected". It will only switch to connected is 
> service hangs at "ready". 
> It's a mistake right? I would rather change that so ready/online service 
> state on local type will set session state to connected. 

I am fine with this.

One of the glitches I see during my testing is when we have two session
one connected to wifi and the other to ethernet, the first one will be
online, the second one only connected.

> Patch 7 introduces the property (getter/setter).
> Patch 8 affect the state and, therefore, the notification behavior according 
> to that connection type.
> 
> Patch 9, 10 and 11 are trivial.
> 
> I tested it as much as I could, through test-session and some custom client 
> app, and it seems to work. I still have things to fix and improve in Session 
> though,
> but getting this patchset reviewed would help a lot before proceeding with 
> next steps.
> 
> Please review,

I like them. Of course there is still a few things to fix but these
patches don't make it worse :)

cheers,
daniel


_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman

Reply via email to