Patrik, I have a few questions regarding your comments to Glenn. I'd like to gain a common understanding so there is less churn and I will ultimately understand the expected behaviour and functionality. I suspect we may be misunderstanding some of your comments. I have limited experience with Gadget behavior so please bear with me.
Based on our HW/testing I see the following behavior. (Admittedly I am limited to one setup--not a ton of exposure) For Cable/Wired Ethernet: Driver probed, no cable connected Powered=TRUE, Connected=false, No Services Cable Connected (No IP Address) Powered=TRUE, Connected=false, No Services Cable Connected (IP Address Assigned) Powered=TRUE, Connected=TRUE, ethernet_MAC_cable This differs slightly from what you mentioned. Ethernet is powered *prior* to carrier detect. For USB Gadget Ethernet: Driver probed, no cable connected Powered=TRUE, Connected=false, No Services Cable Connected (No IP Address) Powered=TRUE, Connected=false, No Services Cable Connected (IP Address Assigned) Powered=TRUE, Connected=TRUE, gadget_MAC_usb >From a high level the two services work the same--as I'd expect. They both use rtnl and get indications when the cables are connected. When the USB cable is connected to the host the DHCP client is started -- no different than Ethernet. My questions is why shouldn't they work the same? They are both a cabled form of Ethernet just different physical layers. I believe from a users point of view they should behave the same. I have some more specific questions to your comments: > The 'ethernet_tethering' flag was used to not to start a DHCP client > when tethering, as detection of ethernet carrier signal meant that the > technology is automatically powered on. Since there is no similar > "carrier" signal for USB gadgets, this part can not be copied over. What are you proposing here? Ethernet indicates "Powered" before even connecting a cable -- no different than our implementation of Gadget Ethernet. As I previously mentioned, we do get a cable connect from rtnl, no different than Ethernet. > Likewise. Ethernets are treated differently. For ethernets plugging in a > cable and detecting carrier equals setting technology powered property > to true manually via D-Bus for the other technologies. No way to > reliably detect an USB cable attached between the USB gadget and host > and the other end USB host commanding us to do ethernet over USB -> no > auto power on or off here. Again, from my testing, Ethernet is "Powered" prior to cable connect (or carrier detect). We do detect the cable connect from rtnl as in Ethernet. Again, why should these be different? Based on my testing the two work the same, are you suggesting they should not? If so, why? 3) Since we are new at this, what is the best work-flow for reworking and splitting up the patches based on your feedback. The only (tedious) approach I can see it starting from a fresh baseline and selectively applying parts of the original patches while doing commits to achieve your suggested grouping. Is there a better/preferred method? Any advice for a newbie? Thanks, Tysen On Fri, Jan 31, 2014 at 9:57 AM, Patrik Flykt <[email protected]>wrote: > On Wed, 2014-01-29 at 09:41 -0500, Glenn Schmottlach wrote: > > From: Glenn Schmottlach <[email protected]> > > > > These changes bring the Linux Gadget support on parity with Ethernet. > > The changes address the use-case where the target platform is a > > USB "device" and is tethered to another host for general connectivity. > > In this situation the target is the device that is tethered. The original > > use-case assumed the platform running Connman provided a bridge or > gateway > > for another device over the Gadget (USB device) interface. > > > > --- > > include/network.h | 1 + > > plugins/ethernet.c | 8 +-- > > plugins/gadget.c | 174 > +++++++++++++++++++++++++++++++++++++++++++++++++++-- > > src/config.c | 20 ++++-- > > src/main.c | 1 + > > src/network.c | 7 +++ > > src/notifier.c | 4 +- > > src/service.c | 25 +++++--- > > src/session.c | 15 +++-- > > src/wispr.c | 2 +- > > 10 files changed, 225 insertions(+), 32 deletions(-) > > > > diff --git a/include/network.h b/include/network.h > > index 3e4cfb4..39ab1f3 100644 > > --- a/include/network.h > > +++ b/include/network.h > > @@ -46,6 +46,7 @@ enum connman_network_type { > > CONNMAN_NETWORK_TYPE_BLUETOOTH_PAN = 8, > > CONNMAN_NETWORK_TYPE_BLUETOOTH_DUN = 9, > > CONNMAN_NETWORK_TYPE_CELLULAR = 10, > > + CONNMAN_NETWORK_TYPE_GADGET = 11, > > CONNMAN_NETWORK_TYPE_VENDOR = 10000, > > }; > > > > diff --git a/plugins/ethernet.c b/plugins/ethernet.c > > index 4c93c4a..ceeac75 100644 > > --- a/plugins/ethernet.c > > +++ b/plugins/ethernet.c > > @@ -324,10 +324,10 @@ static int eth_tech_set_tethering(struct > connman_technology *technology, > > } > > > > static struct connman_technology_driver eth_tech_driver = { > > - .name = > "ethernet", > > - .type = > CONNMAN_SERVICE_TYPE_ETHERNET, > > - .probe = eth_tech_probe, > > - .remove = eth_tech_remove, > > + .name = "ethernet", > > + .type = CONNMAN_SERVICE_TYPE_ETHERNET, > > + .probe = eth_tech_probe, > > + .remove = eth_tech_remove, > > .add_interface = eth_tech_add_interface, > > .remove_interface = eth_tech_remove_interface, > > .set_tethering = eth_tech_set_tethering, > > This part goes to the previous patch, please check the indentation also. > > > diff --git a/plugins/gadget.c b/plugins/gadget.c > > index ace8e9b..1b83080 100644 > > --- a/plugins/gadget.c > > +++ b/plugins/gadget.c > > @@ -36,27 +36,184 @@ > > #include <connman/technology.h> > > #include <connman/plugin.h> > > #include <connman/device.h> > > +#include <connman/rtnl.h> > > #include <connman/inet.h> > > #include <connman/log.h> > > > > +static bool gadget_tethering = false; > > + > > +struct gadget_data { > > + int index; > > + unsigned flags; > > + unsigned int watch; > > + struct connman_network *network; > > +}; > > + > > +static int gadget_network_probe(struct connman_network *network) > > +{ > > + DBG("network %p", network); > > + > > + return 0; > > +} > > + > > +static void gadget_network_remove(struct connman_network *network) > > +{ > > + DBG("network %p", network); > > +} > > + > > +static int gadget_network_connect(struct connman_network *network) > > +{ > > + DBG("network %p", network); > > + > > + connman_network_set_connected(network, true); > > + > > + return 0; > > +} > > + > > +static int gadget_network_disconnect(struct connman_network *network) > > +{ > > + DBG("network %p", network); > > + > > + connman_network_set_connected(network, false); > > + > > + return 0; > > +} > > + > > +static struct connman_network_driver gadget_network_driver = { > > + .name = "usb", > > + .type = CONNMAN_NETWORK_TYPE_GADGET, > > + .probe = gadget_network_probe, > > + .remove = gadget_network_remove, > > + .connect = gadget_network_connect, > > + .disconnect = gadget_network_disconnect, > > +}; > > + > > +static void add_network(struct connman_device *device, > > + struct gadget_data *gadget) > > +{ > > + struct connman_network *network; > > + int index; > > + > > + network = connman_network_create("gadget", > > + CONNMAN_NETWORK_TYPE_GADGET); > > + if (!network) > > + return; > > + > > + index = connman_device_get_index(device); > > + connman_network_set_index(network, index); > > + > > + connman_network_set_name(network, "Wired"); > > + > > + if (connman_device_add_network(device, network) < 0) { > > + connman_network_unref(network); > > + return; > > + } > > + > > + if (!gadget_tethering) > > The 'ethernet_tethering' flag was used to not to start a DHCP client > when tethering, as detection of ethernet carrier signal meant that the > technology is automatically powered on. Since there is no similar > "carrier" signal for USB gadgets, this part can not be copied over. > > > + /* > > + * Prevent service from starting the reconnect > > + * procedure as we do not want the DHCP client > > + * to run when tethering. > > + */ > > + connman_network_set_group(network, "usb"); > > + > > + gadget->network = network; > > +} > > + > > +static void remove_network(struct connman_device *device, > > + struct gadget_data *gadget) > > +{ > > + if (!gadget->network) > > + return; > > + > > + connman_device_remove_network(device, gadget->network); > > + connman_network_unref(gadget->network); > > + > > + gadget->network = NULL; > > +} > > + > > +static void gadget_newlink(unsigned flags, unsigned change, void > *user_data) > > +{ > > + struct connman_device *device = user_data; > > + struct gadget_data *gadget = connman_device_get_data(device); > > + > > + DBG("index %d flags %d change %d", gadget->index, flags, change); > > + > > + if ((gadget->flags & IFF_UP) != (flags & IFF_UP)) { > > + if (flags & IFF_UP) { > > + DBG("power on"); > > + connman_device_set_powered(device, true); > > + } else { > > + DBG("power off"); > > + connman_device_set_powered(device, false); > > Likewise. Ethernets are treated differently. For ethernets plugging in a > cable and detecting carrier equals setting technology powered property > to true manually via D-Bus for the other technologies. No way to > reliably detect an USB cable attached between the USB gadget and host > and the other end USB host commanding us to do ethernet over USB -> no > auto power on or off here. > > > + } > > + } > > + > > + if ((gadget->flags & IFF_LOWER_UP) != (flags & IFF_LOWER_UP)) { > > + if (flags & IFF_LOWER_UP) { > > + DBG("carrier on"); > > + add_network(device, gadget); > > + } else { > > + DBG("carrier off"); > > + remove_network(device, gadget); > > + } > > + } > > + > > + gadget->flags = flags; > > +} > > + > > static int gadget_dev_probe(struct connman_device *device) > > { > > + struct gadget_data *gadget; > > + > > DBG("device %p", device); > > + > > + gadget = g_try_new0(struct gadget_data, 1); > > + if (!gadget) > > + return -ENOMEM; > > + > > + connman_device_set_data(device, gadget); > > + > > + gadget->index = connman_device_get_index(device); > > + gadget->flags = 0; > > + > > + gadget->watch = connman_rtnl_add_newlink_watch(gadget->index, > > + gadget_newlink, device); > > + > > return 0; > > } > > + > > static void gadget_dev_remove(struct connman_device *device) > > { > > + struct gadget_data *gadget = connman_device_get_data(device); > > + > > DBG("device %p", device); > > + > > + connman_device_set_data(device, NULL); > > + > > + connman_rtnl_remove_watch(gadget->watch); > > + > > + remove_network(device, gadget); > > + > > + g_free(gadget); > > } > > + > > static int gadget_dev_enable(struct connman_device *device) > > { > > + struct gadget_data *gadget = connman_device_get_data(device); > > + > > DBG("device %p", device); > > - return 0; > > + > > + return connman_inet_ifup(gadget->index); > > } > > + > > static int gadget_dev_disable(struct connman_device *device) > > { > > + struct gadget_data *gadget = connman_device_get_data(device); > > + > > DBG("device %p", device); > > - return 0; > > + > > + return connman_inet_ifdown(gadget->index); > > } > > > > static struct connman_device_driver gadget_dev_driver = { > > @@ -150,10 +307,10 @@ static void gadget_tech_remove(struct > connman_technology *technology) > > } > > > > static struct connman_technology_driver gadget_tech_driver = { > > - .name = > "cdc_ethernet", > > - .type = > CONNMAN_SERVICE_TYPE_GADGET, > > - .probe = > gadget_tech_probe, > > - .remove = > gadget_tech_remove, > > + .name = "cdc_ethernet", > > + .type = CONNMAN_SERVICE_TYPE_GADGET, > > Indentation...? > > > + .probe = gadget_tech_probe, > > + .remove = gadget_tech_remove, > > .add_interface = gadget_tech_add_interface, > > .remove_interface = gadget_tech_remove_interface, > > .set_tethering = gadget_tech_set_tethering, > > @@ -168,6 +325,10 @@ static int gadget_init(void) > > return err; > > } > > > > + err = connman_network_driver_register(&gadget_network_driver); > > + if (err < 0) > > + return err; > > + > > err = connman_device_driver_register(&gadget_dev_driver); > > if (err < 0) { > > connman_technology_driver_unregister(&gadget_tech_driver); > > @@ -180,6 +341,7 @@ static int gadget_init(void) > > static void gadget_exit(void) > > { > > connman_technology_driver_unregister(&gadget_tech_driver); > > + connman_network_driver_unregister(&gadget_network_driver); > > connman_device_driver_unregister(&gadget_dev_driver); > > } > > Next chunks into 1 (or more) separate patches, please. > > > diff --git a/src/config.c b/src/config.c > > index 19d8b92..799c048 100644 > > --- a/src/config.c > > +++ b/src/config.c > > @@ -182,12 +182,14 @@ static void unregister_service(gpointer data) > > __connman_service_remove(service); > > > > /* > > - * Ethernet service cannot be removed by > > + * Ethernet or gadget service cannot be removed by > > * __connman_service_remove() so reset the ipconfig > > * here. > > */ > > if (connman_service_get_type(service) == > > - > CONNMAN_SERVICE_TYPE_ETHERNET) { > > + > CONNMAN_SERVICE_TYPE_ETHERNET || > > + connman_service_get_type(service) > == > > + > CONNMAN_SERVICE_TYPE_GADGET) { > > __connman_service_disconnect(service); > > __connman_service_reset_ipconfig(service, > > CONNMAN_IPCONFIG_TYPE_IPV4, NULL, > NULL); > > @@ -1072,6 +1074,10 @@ static void provision_service(gpointer key, > gpointer value, > > g_strcmp0(config->type, "ethernet") != 0) > > return; > > > > + if (type == CONNMAN_SERVICE_TYPE_GADGET && > > + g_strcmp0(config->type, "gadget") != 0) > > + return; > > + > > DBG("service %p ident %s", service, > > > __connman_service_get_ident(service)); > > > > @@ -1265,13 +1271,14 @@ int __connman_config_provision_service(struct > connman_service *service) > > GHashTableIter iter; > > gpointer value, key; > > > > - /* For now only WiFi and Ethernet services are supported */ > > + /* For now only WiFi, Gadget and Ethernet services are supported */ > > type = connman_service_get_type(service); > > > > DBG("service %p type %d", service, type); > > > > if (type != CONNMAN_SERVICE_TYPE_WIFI && > > - type != > CONNMAN_SERVICE_TYPE_ETHERNET) > > + type != CONNMAN_SERVICE_TYPE_ETHERNET && > > + type != CONNMAN_SERVICE_TYPE_GADGET) > > return -ENOSYS; > > > > g_hash_table_iter_init(&iter, config_table); > > @@ -1293,13 +1300,14 @@ int > __connman_config_provision_service_ident(struct connman_service *service, > > struct connman_config *config; > > int ret = 0; > > > > - /* For now only WiFi and Ethernet services are supported */ > > + /* For now only WiFi, Gadget and Ethernet services are supported */ > > type = connman_service_get_type(service); > > > > DBG("service %p type %d", service, type); > > > > if (type != CONNMAN_SERVICE_TYPE_WIFI && > > - type != > CONNMAN_SERVICE_TYPE_ETHERNET) > > + type != CONNMAN_SERVICE_TYPE_ETHERNET && > > + type != CONNMAN_SERVICE_TYPE_GADGET) > > return -ENOSYS; > > > > config = g_hash_table_lookup(config_table, ident); > > > diff --git a/src/main.c b/src/main.c > > index e795b52..63da44c 100644 > > --- a/src/main.c > > +++ b/src/main.c > > @@ -48,6 +48,7 @@ > > static char *default_auto_connect[] = { > > "wifi", > > "ethernet", > > + "gadget", > > "cellular", > > NULL > > }; > > Let's be careful with autoconnect here. If the network is already > "detected" without a cable connected, no autoconnect by default. > > > diff --git a/src/network.c b/src/network.c > > index 36366f4..c94f1f8 100644 > > --- a/src/network.c > > +++ b/src/network.c > > @@ -99,6 +99,8 @@ static const char *type2string(enum > connman_network_type type) > > break; > > case CONNMAN_NETWORK_TYPE_ETHERNET: > > return "ethernet"; > > + case CONNMAN_NETWORK_TYPE_GADGET: > > + return "gadget"; > > case CONNMAN_NETWORK_TYPE_WIFI: > > return "wifi"; > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_PAN: > > @@ -791,6 +793,7 @@ static int network_probe(struct connman_network > *network) > > case CONNMAN_NETWORK_TYPE_VENDOR: > > return 0; > > case CONNMAN_NETWORK_TYPE_ETHERNET: > > + case CONNMAN_NETWORK_TYPE_GADGET: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_PAN: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_DUN: > > case CONNMAN_NETWORK_TYPE_CELLULAR: > > @@ -820,6 +823,7 @@ static void network_remove(struct connman_network > *network) > > case CONNMAN_NETWORK_TYPE_VENDOR: > > break; > > case CONNMAN_NETWORK_TYPE_ETHERNET: > > + case CONNMAN_NETWORK_TYPE_GADGET: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_PAN: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_DUN: > > case CONNMAN_NETWORK_TYPE_CELLULAR: > > @@ -1124,6 +1128,7 @@ void connman_network_set_group(struct > connman_network *network, > > case CONNMAN_NETWORK_TYPE_VENDOR: > > return; > > case CONNMAN_NETWORK_TYPE_ETHERNET: > > + case CONNMAN_NETWORK_TYPE_GADGET: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_PAN: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_DUN: > > case CONNMAN_NETWORK_TYPE_CELLULAR: > > @@ -1174,6 +1179,7 @@ bool __connman_network_get_weakness(struct > connman_network *network) > > case CONNMAN_NETWORK_TYPE_UNKNOWN: > > case CONNMAN_NETWORK_TYPE_VENDOR: > > case CONNMAN_NETWORK_TYPE_ETHERNET: > > + case CONNMAN_NETWORK_TYPE_GADGET: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_PAN: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_DUN: > > case CONNMAN_NETWORK_TYPE_CELLULAR: > > @@ -2076,6 +2082,7 @@ void connman_network_update(struct connman_network > *network) > > case CONNMAN_NETWORK_TYPE_VENDOR: > > return; > > case CONNMAN_NETWORK_TYPE_ETHERNET: > > + case CONNMAN_NETWORK_TYPE_GADGET: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_PAN: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_DUN: > > case CONNMAN_NETWORK_TYPE_CELLULAR: > > diff --git a/src/notifier.c b/src/notifier.c > > index 6a601ce..1055b87 100644 > > --- a/src/notifier.c > > +++ b/src/notifier.c > > @@ -146,9 +146,9 @@ void __connman_notifier_connect(enum > connman_service_type type) > > case CONNMAN_SERVICE_TYPE_SYSTEM: > > case CONNMAN_SERVICE_TYPE_GPS: > > case CONNMAN_SERVICE_TYPE_VPN: > > - case CONNMAN_SERVICE_TYPE_GADGET: > > return; > > case CONNMAN_SERVICE_TYPE_ETHERNET: > > + case CONNMAN_SERVICE_TYPE_GADGET: > > case CONNMAN_SERVICE_TYPE_WIFI: > > case CONNMAN_SERVICE_TYPE_BLUETOOTH: > > case CONNMAN_SERVICE_TYPE_CELLULAR: > > @@ -192,9 +192,9 @@ void __connman_notifier_disconnect(enum > connman_service_type type) > > case CONNMAN_SERVICE_TYPE_SYSTEM: > > case CONNMAN_SERVICE_TYPE_GPS: > > case CONNMAN_SERVICE_TYPE_VPN: > > - case CONNMAN_SERVICE_TYPE_GADGET: > > return; > > case CONNMAN_SERVICE_TYPE_ETHERNET: > > + case CONNMAN_SERVICE_TYPE_GADGET: > > case CONNMAN_SERVICE_TYPE_WIFI: > > case CONNMAN_SERVICE_TYPE_BLUETOOTH: > > case CONNMAN_SERVICE_TYPE_CELLULAR: > > diff --git a/src/service.c b/src/service.c > > index 4bc70c0..134f84c 100644 > > --- a/src/service.c > > +++ b/src/service.c > > @@ -344,7 +344,6 @@ static int service_load(struct connman_service > *service) > > case CONNMAN_SERVICE_TYPE_UNKNOWN: > > case CONNMAN_SERVICE_TYPE_SYSTEM: > > case CONNMAN_SERVICE_TYPE_GPS: > > - case CONNMAN_SERVICE_TYPE_GADGET: > > break; > > case CONNMAN_SERVICE_TYPE_VPN: > > service->do_split_routing = g_key_file_get_boolean(keyfile, > > @@ -405,6 +404,7 @@ static int service_load(struct connman_service > *service) > > } > > /* fall through */ > > > > + case CONNMAN_SERVICE_TYPE_GADGET: > > case CONNMAN_SERVICE_TYPE_BLUETOOTH: > > case CONNMAN_SERVICE_TYPE_CELLULAR: > > service->favorite = g_key_file_get_boolean(keyfile, > > @@ -535,7 +535,6 @@ static int service_save(struct connman_service > *service) > > case CONNMAN_SERVICE_TYPE_UNKNOWN: > > case CONNMAN_SERVICE_TYPE_SYSTEM: > > case CONNMAN_SERVICE_TYPE_GPS: > > - case CONNMAN_SERVICE_TYPE_GADGET: > > break; > > case CONNMAN_SERVICE_TYPE_VPN: > > g_key_file_set_boolean(keyfile, service->identifier, > > @@ -579,6 +578,7 @@ static int service_save(struct connman_service > *service) > > } > > /* fall through */ > > > > + case CONNMAN_SERVICE_TYPE_GADGET: > > case CONNMAN_SERVICE_TYPE_BLUETOOTH: > > case CONNMAN_SERVICE_TYPE_CELLULAR: > > g_key_file_set_boolean(keyfile, service->identifier, > > @@ -2260,7 +2260,6 @@ static void append_properties(DBusMessageIter > *dict, dbus_bool_t limited, > > case CONNMAN_SERVICE_TYPE_SYSTEM: > > case CONNMAN_SERVICE_TYPE_GPS: > > case CONNMAN_SERVICE_TYPE_VPN: > > - case CONNMAN_SERVICE_TYPE_GADGET: > > break; > > case CONNMAN_SERVICE_TYPE_CELLULAR: > > val = service->roaming; > > @@ -2273,6 +2272,7 @@ static void append_properties(DBusMessageIter > *dict, dbus_bool_t limited, > > case CONNMAN_SERVICE_TYPE_WIFI: > > case CONNMAN_SERVICE_TYPE_ETHERNET: > > case CONNMAN_SERVICE_TYPE_BLUETOOTH: > > + case CONNMAN_SERVICE_TYPE_GADGET: > > connman_dbus_dict_append_dict(dict, "Ethernet", > > append_ethernet, service); > > break; > > @@ -3472,6 +3472,7 @@ void __connman_service_set_active_session(bool > enable, GSList *list) > > case CONNMAN_SERVICE_TYPE_WIFI: > > case CONNMAN_SERVICE_TYPE_BLUETOOTH: > > case CONNMAN_SERVICE_TYPE_CELLULAR: > > + case CONNMAN_SERVICE_TYPE_GADGET: > > if (enable) > > active_sessions[type]++; > > else > > @@ -3482,18 +3483,18 @@ void __connman_service_set_active_session(bool > enable, GSList *list) > > case CONNMAN_SERVICE_TYPE_SYSTEM: > > case CONNMAN_SERVICE_TYPE_GPS: > > case CONNMAN_SERVICE_TYPE_VPN: > > - case CONNMAN_SERVICE_TYPE_GADGET: > > break; > > } > > > > list = g_slist_next(list); > > } > > > > - DBG("eth %d wifi %d bt %d cellular %d sessions %d", > > + DBG("eth %d wifi %d bt %d cellular %d gadget %d sessions %d", > > active_sessions[CONNMAN_SERVICE_TYPE_ETHERNET], > > active_sessions[CONNMAN_SERVICE_TYPE_WIFI], > > active_sessions[CONNMAN_SERVICE_TYPE_BLUETOOTH], > > active_sessions[CONNMAN_SERVICE_TYPE_CELLULAR], > > + active_sessions[CONNMAN_SERVICE_TYPE_GADGET], > > active_count); > > } > > New patch for the above, it adds gadget support to the rest of the code. > > > @@ -3963,7 +3964,8 @@ static DBusMessage > *disconnect_service(DBusConnection *conn, > > > > bool __connman_service_remove(struct connman_service *service) > > { > > - if (service->type == CONNMAN_SERVICE_TYPE_ETHERNET) > > + if (service->type == CONNMAN_SERVICE_TYPE_ETHERNET || > > + service->type == CONNMAN_SERVICE_TYPE_GADGET) > > return false; > > if (service->immutable || service->hidden || > > Something does not feel quite right immediately with this part. > > > @@ -4657,6 +4659,11 @@ static gint service_compare(gconstpointer a, > gconstpointer b) > > return -1; > > if (service_b->type == CONNMAN_SERVICE_TYPE_VPN) > > return 1; > > + > > + if (service_a->type == CONNMAN_SERVICE_TYPE_GADGET) > > + return -1; > > + if (service_b->type == CONNMAN_SERVICE_TYPE_GADGET) > > + return 1; > > } > > > > strength = (gint) service_b->strength - (gint) service_a->strength; > > Sorting -> new patch. > > > @@ -5739,6 +5746,7 @@ static bool prepare_network(struct connman_service > *service) > > "WiFi.Passphrase", service->passphrase); > > break; > > case CONNMAN_NETWORK_TYPE_ETHERNET: > > + case CONNMAN_NETWORK_TYPE_GADGET: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_PAN: > > case CONNMAN_NETWORK_TYPE_BLUETOOTH_DUN: > > case CONNMAN_NETWORK_TYPE_CELLULAR: > > @@ -5793,9 +5801,9 @@ static int service_connect(struct connman_service > *service) > > case CONNMAN_SERVICE_TYPE_UNKNOWN: > > case CONNMAN_SERVICE_TYPE_SYSTEM: > > case CONNMAN_SERVICE_TYPE_GPS: > > - case CONNMAN_SERVICE_TYPE_GADGET: > > return -EINVAL; > > case CONNMAN_SERVICE_TYPE_ETHERNET: > > + case CONNMAN_SERVICE_TYPE_GADGET: > > case CONNMAN_SERVICE_TYPE_BLUETOOTH: > > case CONNMAN_SERVICE_TYPE_CELLULAR: > > case CONNMAN_SERVICE_TYPE_VPN: > > @@ -5910,7 +5918,6 @@ int __connman_service_connect(struct > connman_service *service) > > case CONNMAN_SERVICE_TYPE_UNKNOWN: > > case CONNMAN_SERVICE_TYPE_SYSTEM: > > case CONNMAN_SERVICE_TYPE_GPS: > > - case CONNMAN_SERVICE_TYPE_GADGET: > > return -EINVAL; > > default: > > if (!is_ipconfig_usable(service)) > > @@ -6521,6 +6528,8 @@ static enum connman_service_type > convert_network_type(struct connman_network *ne > > return CONNMAN_SERVICE_TYPE_BLUETOOTH; > > case CONNMAN_NETWORK_TYPE_CELLULAR: > > return CONNMAN_SERVICE_TYPE_CELLULAR; > > + case CONNMAN_NETWORK_TYPE_GADGET: > > + return CONNMAN_SERVICE_TYPE_GADGET; > > } > > > > return CONNMAN_SERVICE_TYPE_UNKNOWN; > > This together with the patch that introduced service type gadget. > > > diff --git a/src/session.c b/src/session.c > > index 5985a59..c196430 100644 > > --- a/src/session.c > > +++ b/src/session.c > > @@ -193,6 +193,8 @@ static int bearer2service(const char *bearer, enum > connman_service_type *type) > > *type = CONNMAN_SERVICE_TYPE_ETHERNET; > > else if (g_strcmp0(bearer, "wifi") == 0) > > *type = CONNMAN_SERVICE_TYPE_WIFI; > > + else if (g_strcmp0(bearer, "gadget") == 0) > > + *type = CONNMAN_SERVICE_TYPE_GADGET; > > else if (g_strcmp0(bearer, "bluetooth") == 0) > > *type = CONNMAN_SERVICE_TYPE_BLUETOOTH; > > else if (g_strcmp0(bearer, "cellular") == 0) > > @@ -212,6 +214,8 @@ static char *service2bearer(enum > connman_service_type type) > > switch (type) { > > case CONNMAN_SERVICE_TYPE_ETHERNET: > > return "ethernet"; > > + case CONNMAN_SERVICE_TYPE_GADGET: > > + return "gadget"; > > case CONNMAN_SERVICE_TYPE_WIFI: > > return "wifi"; > > case CONNMAN_SERVICE_TYPE_BLUETOOTH: > > @@ -224,7 +228,6 @@ static char *service2bearer(enum > connman_service_type type) > > return "*"; > > case CONNMAN_SERVICE_TYPE_SYSTEM: > > case CONNMAN_SERVICE_TYPE_GPS: > > - case CONNMAN_SERVICE_TYPE_GADGET: > > return ""; > > } > > > > @@ -1050,13 +1053,16 @@ static int service_type_weight(enum > connman_service_type type) > > * according their type. The ordering is > > * > > * 1. Ethernet > > - * 2. Bluetooth > > - * 3. WiFi > > - * 4. Cellular > > + * 2. Gadget > > + * 3. Bluetooth > > + * 4. WiFi > > + * 5. Cellular > > */ > > Ask Wagi where he wants this to go? > > > switch (type) { > > case CONNMAN_SERVICE_TYPE_ETHERNET: > > + return 5; > > + case CONNMAN_SERVICE_TYPE_GADGET: > > return 4; > > case CONNMAN_SERVICE_TYPE_BLUETOOTH: > > return 3; > > @@ -1068,7 +1074,6 @@ static int service_type_weight(enum > connman_service_type type) > > case CONNMAN_SERVICE_TYPE_SYSTEM: > > case CONNMAN_SERVICE_TYPE_GPS: > > case CONNMAN_SERVICE_TYPE_VPN: > > - case CONNMAN_SERVICE_TYPE_GADGET: > > break; > > } > > Session stuff in a patch of its own. > > > diff --git a/src/wispr.c b/src/wispr.c > > index 32cf75a..b5dbf6d 100644 > > --- a/src/wispr.c > > +++ b/src/wispr.c > > @@ -828,12 +828,12 @@ static int wispr_portal_detect(struct > connman_wispr_portal_context *wp_context) > > case CONNMAN_SERVICE_TYPE_WIFI: > > case CONNMAN_SERVICE_TYPE_BLUETOOTH: > > case CONNMAN_SERVICE_TYPE_CELLULAR: > > + case CONNMAN_SERVICE_TYPE_GADGET: > > break; > > case CONNMAN_SERVICE_TYPE_UNKNOWN: > > case CONNMAN_SERVICE_TYPE_SYSTEM: > > case CONNMAN_SERVICE_TYPE_GPS: > > case CONNMAN_SERVICE_TYPE_VPN: > > - case CONNMAN_SERVICE_TYPE_GADGET: > > return -EOPNOTSUPP; > > } > > This one together with the service type support. > > Please split up into more patches as above, let's to round two after > that. There may still be some funny things where a gadget network pops > up immediately if gadget is powered, not sure if we can do anything > about it except fix the USB gadget kernel part to detect a USB host on > the other end perhaps. > > Cheers, > > Patrik > > > _______________________________________________ > connman mailing list > [email protected] > https://lists.connman.net/mailman/listinfo/connman > _______________________________________________ connman mailing list [email protected] https://lists.connman.net/mailman/listinfo/connman
