Re: [PATCH] hso: Set modem name based on udev network interface name

Re: [PATCH] hso: Set modem name based on udev network interface name

[=?unknown-8bit?q?Beno=C3=AEt?= Monin]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

Hi Denis,

> De : Denis Kenzior
> 
> Hi Benoît,
> 
> I prefer you do this in plugins/hso and not in udev.
> 
Is the patch below correct ? I moved the call to set_name
to hso_probe.

Benoît
--
Subject: [PATCH] hso: Set modem name based on udev network interface name

Useful if you have multiple identical modems and you need a way
to know which modem you're talking to.
---
 plugins/hso.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/plugins/hso.c b/plugins/hso.c
index 055c63b..a9fb1ba 100644
--- a/plugins/hso.c
+++ b/plugins/hso.c
@@ -59,6 +59,7 @@ struct hso_data {
 static int hso_probe(struct ofono_modem *modem)
 {
        struct hso_data *data;
+       const char *interface;

        DBG("%p", modem);

@@ -68,6 +69,11 @@ static int hso_probe(struct ofono_modem *modem)

        ofono_modem_set_data(modem, data);

+       /* Set name based on network interface name */
+       interface = ofono_modem_get_string(modem, "NetworkInterface");
+       if (interface)
+               ofono_modem_set_name(modem, interface);
+
        return 0;
 }

--

Re: [PATCH] hso: Set modem name based on udev network interface name

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1270 bytes --]

Hi Benoit,

> Subject: [PATCH] hso: Set modem name based on udev network interface name
> 
> Useful if you have multiple identical modems and you need a way
> to know which modem you're talking to.
> ---
>  plugins/hso.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/plugins/hso.c b/plugins/hso.c
> index 055c63b..a9fb1ba 100644
> --- a/plugins/hso.c
> +++ b/plugins/hso.c
> @@ -59,6 +59,7 @@ struct hso_data {
>  static int hso_probe(struct ofono_modem *modem)
>  {
>         struct hso_data *data;
> +       const char *interface;
> 
>         DBG("%p", modem);
> 
> @@ -68,6 +69,11 @@ static int hso_probe(struct ofono_modem *modem)
> 
>         ofono_modem_set_data(modem, data);
> 
> +       /* Set name based on network interface name */
> +       interface = ofono_modem_get_string(modem, "NetworkInterface");
> +       if (interface)
> +               ofono_modem_set_name(modem, interface);
> +
>         return 0;
>  }

the friendly modem name is for devices that do have a friendly name,
like Bluetooth headsets. Why do you wanna misuse the interface name
here. I am a bit against this, because it leads to speculation and
assumptions in userspace programs.

Regards

Marcel

Re: [PATCH] hso: Set modem name based on udev network interface name

[=?unknown-8bit?q?Beno=C3=AEt?= Monin]

[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]

Hi Marcel,

> De : Marcel Holtmann
> 
> Hi Benoit,
> 
> the friendly modem name is for devices that do have a friendly name,
> like Bluetooth headsets. Why do you wanna misuse the interface name
> here. I am a bit against this, because it leads to speculation and
> assumptions in userspace programs.
> 
Maybe I should give some details of what I'm trying to achieve. The
device we're building has 2 identical modems with fixed position on
USB. We also have the possibility to drive their power supply
independently and to swap the sim cards.

I need a way to know which one is modem A and which one is modem B
and I have no easy way of knowing the modem serial number or the
sim subscriber id beforehand. With an udev rule, I can easily set
a fixed name for the network interface of each modem. But under
ofono, I can't tell which modem is which. That's why I added this
"not-so-friendly" name.

Is there a way of knowing which physical modem ofono is refering to ?
I recognize that our case may not be the most common and I understand
that you don't want to abuse the Name property. Maybe exposing the
NetworkInterface as a modem property would be ok ? or exposing
a PhysicalName ?

--
Benoît.

Re: [PATCH] hso: Set modem name based on udev network interface name

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

Hi Benoit,

please fix your mail client. You keep breaking threading every single
time you respond. That is pretty annoying.

> > the friendly modem name is for devices that do have a friendly name,
> > like Bluetooth headsets. Why do you wanna misuse the interface name
> > here. I am a bit against this, because it leads to speculation and
> > assumptions in userspace programs.
> > 
> Maybe I should give some details of what I'm trying to achieve. The
> device we're building has 2 identical modems with fixed position on
> USB. We also have the possibility to drive their power supply
> independently and to swap the sim cards.
> 
> I need a way to know which one is modem A and which one is modem B
> and I have no easy way of knowing the modem serial number or the
> sim subscriber id beforehand. With an udev rule, I can easily set
> a fixed name for the network interface of each modem. But under
> ofono, I can't tell which modem is which. That's why I added this
> "not-so-friendly" name.

actually do you get the serial number from the USB descriptors?

> Is there a way of knowing which physical modem ofono is refering to ?
> I recognize that our case may not be the most common and I understand
> that you don't want to abuse the Name property. Maybe exposing the
> NetworkInterface as a modem property would be ok ? or exposing
> a PhysicalName ?

oFono does not have the concept of physical devices actually. We moved
away from dealing with specific modem hardware for a reason. oFono is
based around a SIM card.

However we did allow the object path of the modem to be unique, but for
that, the udev plugin needs to know it. And in your case the modem needs
to have proper USB descriptors for that.

Regards

Marcel

Re: [PATCH] hso: Set modem name based on udev network interface name

[=?unknown-8bit?q?Beno=C3=AEt?= Monin]

[-- Attachment #1: Type: text/plain, Size: 915 bytes --]

Hi Marcel,
[Sorry for the delay]

Le lundi 25 octobre 2010 14:15:55 Marcel Holtmann, vous avez écrit :
> actually do you get the serial number from the USB descriptors?
> 
No, the serial number comes from an AT command, I guess.

> oFono does not have the concept of physical devices actually. We moved
> away from dealing with specific modem hardware for a reason. oFono is
> based around a SIM card.
> 
> However we did allow the object path of the modem to be unique, but for
> that, the udev plugin needs to know it. And in your case the modem needs
> to have proper USB descriptors for that.
> 
I checked with udevadm info, ID_SERIAL_SHORT is not set and ID_SERIAL
is "Option_N.V._Globetrotter_HSUPA_Modem" for both modem. So I simply set
ID_SERIAL_SHORT to what I wanted with an udev rule. 

Thank you for pointing me to udev and please discard my patch.

--
Cordially,
Benoît.

[PATCH] hso: Set modem name based on udev network interface name

[=?unknown-8bit?q?Beno=C3=AEt?= Monin]

[-- Attachment #1: Type: text/plain, Size: 740 bytes --]

Useful if you have multiple identical modems and you need a way
to know which modem you're talking to.
---
 plugins/udev.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/plugins/udev.c b/plugins/udev.c
index 737a637..e687c01 100644
--- a/plugins/udev.c
+++ b/plugins/udev.c
@@ -191,6 +191,7 @@ static void add_hso(struct ofono_modem *modem,
        } else if (g_str_equal(subsystem, "net") == TRUE) {
                devnode = get_property(udev_device, "INTERFACE");
                ofono_modem_set_string(modem, NETWORK_INTERFACE, devnode);
+               ofono_modem_set_name(modem, devnode);
        } else {
                return;
        }
--

Re: [PATCH] hso: Set modem name based on udev network interface name

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

Hi Benoît,

On 10/21/2010 03:26 AM, "Benoît Monin" wrote:
> Useful if you have multiple identical modems and you need a way
> to know which modem you're talking to.
> ---
>  plugins/udev.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/plugins/udev.c b/plugins/udev.c
> index 737a637..e687c01 100644
> --- a/plugins/udev.c
> +++ b/plugins/udev.c
> @@ -191,6 +191,7 @@ static void add_hso(struct ofono_modem *modem,
>         } else if (g_str_equal(subsystem, "net") == TRUE) {
>                 devnode = get_property(udev_device, "INTERFACE");
>                 ofono_modem_set_string(modem, NETWORK_INTERFACE, devnode);
> +               ofono_modem_set_name(modem, devnode);

I prefer you do this in plugins/hso and not in udev.

Regards,
-Denis