Hi Patrik, On 08/30/2013 01:00 PM, Patrik Flykt wrote:
Hi,The first patch is a cleanup for the current autoconnect behavior. It makes the code more readable and does not change any functionality nor is it needed for the rest of the code. The second patch is a fix for the test case where the session code nowadays creates unique session paths. Patch 03 updates the test cases as sessions 0 and 1 should really be independent of each other. Patch 04 provides a function for counting "active" sessions, that is sessions that have called Connect() to indicate they really would like to have their bearer types connected. Patch 04 calls this function from the session code for these "active" sessions. Updates via D-Bus and policy plugins are taken into account. Patch 06 reorganizes auto_connect_service() code. This reorganization is done to show that the initial changes in this patch do not change autoconnection behavior and prepares for easier addition of the session changes in patch 07. Patch 07 uses the "active" session information when autoconnecting. With "active" sessions, the autoconnection mechanism does not stop at the first connected service but continues to the next service type. When it is known that a service type, a.k.a. bearer, is connecting/connected, all other services of this type are ignored. Similarly when a service is found and connection initiated, the autconnection mechanism will continue to the next service type, if any. Patch 08 removes direct calls to service connect and disconnect and replaces them with a strategically placed call to autoconnect instead. At this point it no longer makes sense to store all services in all sessions and patch 10 will add only connected services to sessions. A helper function for this is added as patch 09. SessionMode is gone with patch 11 except that the property is kept for compatibility reasons. Documentation is also updated (patch 12). Services were reference counted by session, but those functions are no longer needed and removed in patches 13 and 14. Finally code for pending functions can be removed with patch 15, as it is not needed any longer either. With these modifications the Session test tool will run fine, except for the last session policy part where I lacked kernel support for making it work. Also the limited amount of banging with connmanctl session ... seemed to work fine.
Thanks, this looks really awesome. Nice simplification.
What is left to do is to clean up the selection and deselection code in session.c. Most of it can probably be simplified away. As there are not too many services per session, maybe the session service hash is unnecessary and only the existing sorted list would be enough.
Yes, if I got this right we might have only one bearer at maximum. Well, in the case of VPN it is not true but we do not allow to have a VPN as bearer type at this moment. Well one thing I ponder on is how to support 'Select Services by Name' and 'Filter out Services by Name' for a session. I know we do not support it upstream yet. Meanwhile I have a bunch of patches for this here: https://github.com/bmwcarit/connman/tree/policy-services-v4 Any ideas how we could support this in the future?
And yes, the commit messages might need some more explanations in them.
Yes, that wouldn't hurt :)
Needless to say, please review.
Apart of my nitpicking it looks pretty good to me. cheers, daniel _______________________________________________ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
