---
profiles/audio/avctp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 70a3e40b2..ae53587ad 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -1181,8 +1181,7 @@ static int uinput_create(char *name)
}
memset(&dev, 0, sizeof(dev));
- if (name)
- strncpy(dev.name, name, UINPUT_MAX_NAME_SIZE - 1);
+ snprintf(dev.name, sizeof(dev.name), "Bluetooth Audio/Video Remote Control%s%s", name ? " " : "", name ? name : "");
dev.id.bustype = BUS_BLUETOOTH;
dev.id.vendor = 0x0000;
--
2.11.0
Hi Pali,
On Tue, Nov 26, 2019 at 11:31 PM Pali Rohár <[email protected]> wrote:
>
> ---
> profiles/audio/avctp.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> index 70a3e40b2..ae53587ad 100644
> --- a/profiles/audio/avctp.c
> +++ b/profiles/audio/avctp.c
> @@ -1181,8 +1181,7 @@ static int uinput_create(char *name)
> }
>
> memset(&dev, 0, sizeof(dev));
> - if (name)
> - strncpy(dev.name, name, UINPUT_MAX_NAME_SIZE - 1);
> + snprintf(dev.name, sizeof(dev.name), "Bluetooth Audio/Video Remote Control%s%s", name ? " " : "", name ? name : "");
>
> dev.id.bustype = BUS_BLUETOOTH;
> dev.id.vendor = 0x0000;
> --
> 2.11.0
It is already setting a bustype to BUS_BLUETOOTH why do you need to
make it more specific, btw it needs to be limited to
UINPUT_MAX_NAME_SIZE. Id say if we want to make it declare the
connection type that probably something that needs to be encoded in
the bus itself, like BUS_BLUETOOTH_AVCTP, BUS_BLUETOOTH_HOG, etc.
--
Luiz Augusto von Dentz
Hi!
On Wednesday 27 November 2019 17:50:40 Luiz Augusto von Dentz wrote:
> Hi Pali,
>
> On Tue, Nov 26, 2019 at 11:31 PM Pali Rohár <[email protected]> wrote:
> >
> > ---
> > profiles/audio/avctp.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> > index 70a3e40b2..ae53587ad 100644
> > --- a/profiles/audio/avctp.c
> > +++ b/profiles/audio/avctp.c
> > @@ -1181,8 +1181,7 @@ static int uinput_create(char *name)
> > }
> >
> > memset(&dev, 0, sizeof(dev));
> > - if (name)
> > - strncpy(dev.name, name, UINPUT_MAX_NAME_SIZE - 1);
> > + snprintf(dev.name, sizeof(dev.name), "Bluetooth Audio/Video Remote Control%s%s", name ? " " : "", name ? name : "");
> >
> > dev.id.bustype = BUS_BLUETOOTH;
> > dev.id.vendor = 0x0000;
> > --
> > 2.11.0
>
> It is already setting a bustype to BUS_BLUETOOTH why do you need to
> make it more specific,
Because more tools shows only device name. Also in kernel dmesg is only
device name. And MAC address is really something which does not say
anything about device...
For this reason also older input devices (implemented in kernel) exports
more descriptive names, e.g. "AT Translated Set 2 keyboard" or "ETPS/2
Elantech TrackPoint" or "Integrated IR Camera".
> btw it needs to be limited to UINPUT_MAX_NAME_SIZE.
Is not this implicitly defined by sizeof()?
> Id say if we want to make it declare the connection type
Yes, connection type would be really useful.
Now I'm playing with exporting "button press even" from HSP profile via
uinput device too and therefore connection type should be there.
> that probably something that needs to be encoded in
> the bus itself, like BUS_BLUETOOTH_AVCTP, BUS_BLUETOOTH_HOG, etc.
But bus type is kernel API and there is no such AVCTP or HOG. So we need
to stick with BUS_BLUETOOH.
Other kernel devices also put protocol information into name (e.g.
touchpad / trackpoints), so it is ideal place also for bluetooth
devices.
And probably most userspace devices are mapped to BUS_VIRTUAL. There
are no subtypes.
--
Pali Rohár
[email protected]
On Wed, Nov 27, 2019 at 8:07 AM Pali Rohár <[email protected]> wrote:
>
> Hi!
>
> On Wednesday 27 November 2019 17:50:40 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> >
> > On Tue, Nov 26, 2019 at 11:31 PM Pali Rohár <[email protected]> wrote:
> > >
> > > ---
> > > profiles/audio/avctp.c | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> > > index 70a3e40b2..ae53587ad 100644
> > > --- a/profiles/audio/avctp.c
> > > +++ b/profiles/audio/avctp.c
> > > @@ -1181,8 +1181,7 @@ static int uinput_create(char *name)
> > > }
> > >
> > > memset(&dev, 0, sizeof(dev));
> > > - if (name)
> > > - strncpy(dev.name, name, UINPUT_MAX_NAME_SIZE - 1);
> > > + snprintf(dev.name, sizeof(dev.name), "Bluetooth Audio/Video Remote Control%s%s", name ? " " : "", name ? name : "");
> > >
> > > dev.id.bustype = BUS_BLUETOOTH;
> > > dev.id.vendor = 0x0000;
> > > --
> > > 2.11.0
> >
> > It is already setting a bustype to BUS_BLUETOOTH why do you need to
> > make it more specific,
>
> Because more tools shows only device name. Also in kernel dmesg is only
> device name. And MAC address is really something which does not say
> anything about device...
>
> For this reason also older input devices (implemented in kernel) exports
> more descriptive names, e.g. "AT Translated Set 2 keyboard" or "ETPS/2
> Elantech TrackPoint" or "Integrated IR Camera".
>
> > btw it needs to be limited to UINPUT_MAX_NAME_SIZE.
>
> Is not this implicitly defined by sizeof()?
>
> > Id say if we want to make it declare the connection type
>
> Yes, connection type would be really useful.
>
> Now I'm playing with exporting "button press even" from HSP profile via
> uinput device too and therefore connection type should be there.
>
> > that probably something that needs to be encoded in
> > the bus itself, like BUS_BLUETOOTH_AVCTP, BUS_BLUETOOTH_HOG, etc.
>
> But bus type is kernel API and there is no such AVCTP or HOG. So we need
> to stick with BUS_BLUETOOH.
>
> Other kernel devices also put protocol information into name (e.g.
> touchpad / trackpoints), so it is ideal place also for bluetooth
> devices.
>
> And probably most userspace devices are mapped to BUS_VIRTUAL. There
> are no subtypes.
>
> --
> Pali Rohár
> [email protected]
Wouldn't it be better to use the device name for uinput rather than the address?
In `init_uinput` (avctp.c), uinput_create is called with
device_get_address instead of device_get_name. It would probably be
better to use the device name for the uinput and set the device
address as the `uniq` attribute (as it is done for /dev/uhid for HID
devices).
So in the following, replace ATTR{name} with something like "Some
Company Headphones", ATTR{uniq} = "33:33:33:ff:ff:ff" and ATTR{phys}
with the controller's address.
$ udevadm info -a -p /sys/devices/virtual/input/input21/
...
looking at device '/devices/virtual/input/input21':
KERNEL=="input21"
SUBSYSTEM=="input"
DRIVER==""
ATTR{inhibited}=="0"
ATTR{name}=="33:33:33:FF:FF:FF"
ATTR{phys}==""
ATTR{properties}=="0"
ATTR{uniq}==""
This is what uhid looks like in comparison:
$ udevadm info -a -p /sys/class/misc/uhid/0005\:046D\:B33B.0002/input/input18/
looking at device
'/devices/virtual/misc/uhid/0005:046D:B33B.0002/input/input18':
KERNEL=="input17"
SUBSYSTEM=="input"
DRIVER==""
ATTR{inhibited}=="0"
ATTR{name}=="Keyboard K780 Keyboard"
ATTR{phys}=="ab:cd:ef:12:34:56"
ATTR{properties}=="0"
ATTR{uniq}=="33:33:33:33:ff:ff"
Abhishek
On Wednesday 27 November 2019 09:21:24 Abhishek Pandit-Subedi wrote:
> On Wed, Nov 27, 2019 at 8:07 AM Pali Rohár <[email protected]> wrote:
> >
> > Hi!
> >
> > On Wednesday 27 November 2019 17:50:40 Luiz Augusto von Dentz wrote:
> > > Hi Pali,
> > >
> > > On Tue, Nov 26, 2019 at 11:31 PM Pali Rohár <[email protected]> wrote:
> > > >
> > > > ---
> > > > profiles/audio/avctp.c | 3 +--
> > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> > > > index 70a3e40b2..ae53587ad 100644
> > > > --- a/profiles/audio/avctp.c
> > > > +++ b/profiles/audio/avctp.c
> > > > @@ -1181,8 +1181,7 @@ static int uinput_create(char *name)
> > > > }
> > > >
> > > > memset(&dev, 0, sizeof(dev));
> > > > - if (name)
> > > > - strncpy(dev.name, name, UINPUT_MAX_NAME_SIZE - 1);
> > > > + snprintf(dev.name, sizeof(dev.name), "Bluetooth Audio/Video Remote Control%s%s", name ? " " : "", name ? name : "");
> > > >
> > > > dev.id.bustype = BUS_BLUETOOTH;
> > > > dev.id.vendor = 0x0000;
> > > > --
> > > > 2.11.0
> > >
> > > It is already setting a bustype to BUS_BLUETOOTH why do you need to
> > > make it more specific,
> >
> > Because more tools shows only device name. Also in kernel dmesg is only
> > device name. And MAC address is really something which does not say
> > anything about device...
> >
> > For this reason also older input devices (implemented in kernel) exports
> > more descriptive names, e.g. "AT Translated Set 2 keyboard" or "ETPS/2
> > Elantech TrackPoint" or "Integrated IR Camera".
> >
> > > btw it needs to be limited to UINPUT_MAX_NAME_SIZE.
> >
> > Is not this implicitly defined by sizeof()?
> >
> > > Id say if we want to make it declare the connection type
> >
> > Yes, connection type would be really useful.
> >
> > Now I'm playing with exporting "button press even" from HSP profile via
> > uinput device too and therefore connection type should be there.
> >
> > > that probably something that needs to be encoded in
> > > the bus itself, like BUS_BLUETOOTH_AVCTP, BUS_BLUETOOTH_HOG, etc.
> >
> > But bus type is kernel API and there is no such AVCTP or HOG. So we need
> > to stick with BUS_BLUETOOH.
> >
> > Other kernel devices also put protocol information into name (e.g.
> > touchpad / trackpoints), so it is ideal place also for bluetooth
> > devices.
> >
> > And probably most userspace devices are mapped to BUS_VIRTUAL. There
> > are no subtypes.
> >
> > --
> > Pali Rohár
> > [email protected]
>
> Wouldn't it be better to use the device name for uinput rather than the address?
Hi!
Can we access device name in bluez? If yes that would be improvement too!
But because more profiles can export input device we still should
include profile name (or abbrev) into device name.
> In `init_uinput` (avctp.c), uinput_create is called with
> device_get_address instead of device_get_name. It would probably be
> better to use the device name for the uinput and set the device
> address as the `uniq` attribute (as it is done for /dev/uhid for HID
> devices).
And how to set uniq attribute? I'm looking at uinput API, but do not see
any way how to set such thing.
According to uinput kernel documentation there are two ways how to setup
uinput device. New one via UI_DEV_SETUP ioctl and the old one by writing
structure via write() syscall.
https://www.kernel.org/doc/html/latest/input/uinput.html
New way which has to call UI_DEV_SETUP ioctl has following struct
uinput_setup members:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/uinput.h#n67
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/input.h#n58
u16 bustype;
u16 vendor;
u16 product;
u16 version;
char name[UINPUT_MAX_NAME_SIZE];
u32 ff_effects_max;
Old way how to setup uinput is to call write() systecall with struct
uinput_user_dev members, which are:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/uinput.h#n223
char name[UINPUT_MAX_NAME_SIZE];
u16 bustype;
u16 vendor;
u16 product;
u16 version;
u32 ff_effects_max;
s32 absmax[ABS_CNT];
s32 absmin[ABS_CNT];
s32 absfuzz[ABS_CNT];
s32 absflat[ABS_CNT];
And I do not see any field which could represents that "uniq" attribute.
Seems that it can be exported only by kernel input drivers, not uinput
userspace drivers...
> So in the following, replace ATTR{name} with something like "Some
> Company Headphones", ATTR{uniq} = "33:33:33:ff:ff:ff" and ATTR{phys}
> with the controller's address.
>
> $ udevadm info -a -p /sys/devices/virtual/input/input21/
> ...
> looking at device '/devices/virtual/input/input21':
> KERNEL=="input21"
> SUBSYSTEM=="input"
> DRIVER==""
> ATTR{inhibited}=="0"
> ATTR{name}=="33:33:33:FF:FF:FF"
> ATTR{phys}==""
> ATTR{properties}=="0"
> ATTR{uniq}==""
>
> This is what uhid looks like in comparison:
>
> $ udevadm info -a -p /sys/class/misc/uhid/0005\:046D\:B33B.0002/input/input18/
>
> looking at device
> '/devices/virtual/misc/uhid/0005:046D:B33B.0002/input/input18':
> KERNEL=="input17"
> SUBSYSTEM=="input"
> DRIVER==""
> ATTR{inhibited}=="0"
> ATTR{name}=="Keyboard K780 Keyboard"
> ATTR{phys}=="ab:cd:ef:12:34:56"
> ATTR{properties}=="0"
> ATTR{uniq}=="33:33:33:33:ff:ff"
>
>
> Abhishek
--
Pali Rohár
[email protected]
On Wed, Nov 27, 2019 at 9:36 AM Pali Rohár <[email protected]> wrote:
>
> On Wednesday 27 November 2019 09:21:24 Abhishek Pandit-Subedi wrote:
> > On Wed, Nov 27, 2019 at 8:07 AM Pali Rohár <[email protected]> wrote:
> > >
> > > Hi!
> > >
> > > On Wednesday 27 November 2019 17:50:40 Luiz Augusto von Dentz wrote:
> > > > Hi Pali,
> > > >
> > > > On Tue, Nov 26, 2019 at 11:31 PM Pali Rohár <[email protected]> wrote:
> > > > >
> > > > > ---
> > > > > profiles/audio/avctp.c | 3 +--
> > > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> > > > > index 70a3e40b2..ae53587ad 100644
> > > > > --- a/profiles/audio/avctp.c
> > > > > +++ b/profiles/audio/avctp.c
> > > > > @@ -1181,8 +1181,7 @@ static int uinput_create(char *name)
> > > > > }
> > > > >
> > > > > memset(&dev, 0, sizeof(dev));
> > > > > - if (name)
> > > > > - strncpy(dev.name, name, UINPUT_MAX_NAME_SIZE - 1);
> > > > > + snprintf(dev.name, sizeof(dev.name), "Bluetooth Audio/Video Remote Control%s%s", name ? " " : "", name ? name : "");
> > > > >
> > > > > dev.id.bustype = BUS_BLUETOOTH;
> > > > > dev.id.vendor = 0x0000;
> > > > > --
> > > > > 2.11.0
> > > >
> > > > It is already setting a bustype to BUS_BLUETOOTH why do you need to
> > > > make it more specific,
> > >
> > > Because more tools shows only device name. Also in kernel dmesg is only
> > > device name. And MAC address is really something which does not say
> > > anything about device...
> > >
> > > For this reason also older input devices (implemented in kernel) exports
> > > more descriptive names, e.g. "AT Translated Set 2 keyboard" or "ETPS/2
> > > Elantech TrackPoint" or "Integrated IR Camera".
> > >
> > > > btw it needs to be limited to UINPUT_MAX_NAME_SIZE.
> > >
> > > Is not this implicitly defined by sizeof()?
> > >
> > > > Id say if we want to make it declare the connection type
> > >
> > > Yes, connection type would be really useful.
> > >
> > > Now I'm playing with exporting "button press even" from HSP profile via
> > > uinput device too and therefore connection type should be there.
> > >
> > > > that probably something that needs to be encoded in
> > > > the bus itself, like BUS_BLUETOOTH_AVCTP, BUS_BLUETOOTH_HOG, etc.
> > >
> > > But bus type is kernel API and there is no such AVCTP or HOG. So we need
> > > to stick with BUS_BLUETOOH.
> > >
> > > Other kernel devices also put protocol information into name (e.g.
> > > touchpad / trackpoints), so it is ideal place also for bluetooth
> > > devices.
> > >
> > > And probably most userspace devices are mapped to BUS_VIRTUAL. There
> > > are no subtypes.
> > >
> > > --
> > > Pali Rohár
> > > [email protected]
> >
> > Wouldn't it be better to use the device name for uinput rather than the address?
>
> Hi!
>
> Can we access device name in bluez? If yes that would be improvement too!
>
> But because more profiles can export input device we still should
> include profile name (or abbrev) into device name.
Yup, I would be totally ok with doing something like "Your Device Name
Here (AVCTP)"
>
> > In `init_uinput` (avctp.c), uinput_create is called with
> > device_get_address instead of device_get_name. It would probably be
> > better to use the device name for the uinput and set the device
> > address as the `uniq` attribute (as it is done for /dev/uhid for HID
> > devices).
>
> And how to set uniq attribute? I'm looking at uinput API, but do not see
> any way how to set such thing.
>
> According to uinput kernel documentation there are two ways how to setup
> uinput device. New one via UI_DEV_SETUP ioctl and the old one by writing
> structure via write() syscall.
>
> https://www.kernel.org/doc/html/latest/input/uinput.html
>
> New way which has to call UI_DEV_SETUP ioctl has following struct
> uinput_setup members:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/uinput.h#n67
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/input.h#n58
>
> u16 bustype;
> u16 vendor;
> u16 product;
> u16 version;
> char name[UINPUT_MAX_NAME_SIZE];
> u32 ff_effects_max;
>
> Old way how to setup uinput is to call write() systecall with struct
> uinput_user_dev members, which are:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/uinput.h#n223
>
> char name[UINPUT_MAX_NAME_SIZE];
> u16 bustype;
> u16 vendor;
> u16 product;
> u16 version;
> u32 ff_effects_max;
> s32 absmax[ABS_CNT];
> s32 absmin[ABS_CNT];
> s32 absfuzz[ABS_CNT];
> s32 absflat[ABS_CNT];
>
> And I do not see any field which could represents that "uniq" attribute.
> Seems that it can be exported only by kernel input drivers, not uinput
> userspace drivers...
You are correct -- setting uniq is currently missing in uinput.h.
(https://github.com/torvalds/linux/blob/6f0d349d922ba44e4348a17a78ea51b7135965b1/include/uapi/linux/uinput.h#L145
I'm working on a patch to fix that and will send it to
linux-bluetooth@ mailing list later today.
Setting ATTR{phys} is possible via ioctl(fd, UI_SET_PHYS, phys) and I
sent a patch to do that yesterday:
https://lore.kernel.org/linux-bluetooth/20191126160013.BlueZ.1.I95600043ffb5b2b6c4782497ff735ab5d3e37c13@changeid/T/#u
>
> > So in the following, replace ATTR{name} with something like "Some
> > Company Headphones", ATTR{uniq} = "33:33:33:ff:ff:ff" and ATTR{phys}
> > with the controller's address.
> >
> > $ udevadm info -a -p /sys/devices/virtual/input/input21/
> > ...
> > looking at device '/devices/virtual/input/input21':
> > KERNEL=="input21"
> > SUBSYSTEM=="input"
> > DRIVER==""
> > ATTR{inhibited}=="0"
> > ATTR{name}=="33:33:33:FF:FF:FF"
> > ATTR{phys}==""
> > ATTR{properties}=="0"
> > ATTR{uniq}==""
> >
> > This is what uhid looks like in comparison:
> >
> > $ udevadm info -a -p /sys/class/misc/uhid/0005\:046D\:B33B.0002/input/input18/
> >
> > looking at device
> > '/devices/virtual/misc/uhid/0005:046D:B33B.0002/input/input18':
> > KERNEL=="input17"
> > SUBSYSTEM=="input"
> > DRIVER==""
> > ATTR{inhibited}=="0"
> > ATTR{name}=="Keyboard K780 Keyboard"
> > ATTR{phys}=="ab:cd:ef:12:34:56"
> > ATTR{properties}=="0"
> > ATTR{uniq}=="33:33:33:33:ff:ff"
> >
> >
> > Abhishek
>
> --
> Pali Rohár
> [email protected]
On Wednesday 27 November 2019 09:45:32 Abhishek Pandit-Subedi wrote:
> On Wed, Nov 27, 2019 at 9:36 AM Pali Rohár <[email protected]> wrote:
> >
> > On Wednesday 27 November 2019 09:21:24 Abhishek Pandit-Subedi wrote:
> > > On Wed, Nov 27, 2019 at 8:07 AM Pali Rohár <[email protected]> wrote:
> > > >
> > > > Hi!
> > > >
> > > > On Wednesday 27 November 2019 17:50:40 Luiz Augusto von Dentz wrote:
> > > > > Hi Pali,
> > > > >
> > > > > On Tue, Nov 26, 2019 at 11:31 PM Pali Rohár <[email protected]> wrote:
> > > > > >
> > > > > > ---
> > > > > > profiles/audio/avctp.c | 3 +--
> > > > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> > > > > > index 70a3e40b2..ae53587ad 100644
> > > > > > --- a/profiles/audio/avctp.c
> > > > > > +++ b/profiles/audio/avctp.c
> > > > > > @@ -1181,8 +1181,7 @@ static int uinput_create(char *name)
> > > > > > }
> > > > > >
> > > > > > memset(&dev, 0, sizeof(dev));
> > > > > > - if (name)
> > > > > > - strncpy(dev.name, name, UINPUT_MAX_NAME_SIZE - 1);
> > > > > > + snprintf(dev.name, sizeof(dev.name), "Bluetooth Audio/Video Remote Control%s%s", name ? " " : "", name ? name : "");
> > > > > >
> > > > > > dev.id.bustype = BUS_BLUETOOTH;
> > > > > > dev.id.vendor = 0x0000;
> > > > > > --
> > > > > > 2.11.0
> > > > >
> > > > > It is already setting a bustype to BUS_BLUETOOTH why do you need to
> > > > > make it more specific,
> > > >
> > > > Because more tools shows only device name. Also in kernel dmesg is only
> > > > device name. And MAC address is really something which does not say
> > > > anything about device...
> > > >
> > > > For this reason also older input devices (implemented in kernel) exports
> > > > more descriptive names, e.g. "AT Translated Set 2 keyboard" or "ETPS/2
> > > > Elantech TrackPoint" or "Integrated IR Camera".
> > > >
> > > > > btw it needs to be limited to UINPUT_MAX_NAME_SIZE.
> > > >
> > > > Is not this implicitly defined by sizeof()?
> > > >
> > > > > Id say if we want to make it declare the connection type
> > > >
> > > > Yes, connection type would be really useful.
> > > >
> > > > Now I'm playing with exporting "button press even" from HSP profile via
> > > > uinput device too and therefore connection type should be there.
> > > >
> > > > > that probably something that needs to be encoded in
> > > > > the bus itself, like BUS_BLUETOOTH_AVCTP, BUS_BLUETOOTH_HOG, etc.
> > > >
> > > > But bus type is kernel API and there is no such AVCTP or HOG. So we need
> > > > to stick with BUS_BLUETOOH.
> > > >
> > > > Other kernel devices also put protocol information into name (e.g.
> > > > touchpad / trackpoints), so it is ideal place also for bluetooth
> > > > devices.
> > > >
> > > > And probably most userspace devices are mapped to BUS_VIRTUAL. There
> > > > are no subtypes.
> > > >
> > > > --
> > > > Pali Rohár
> > > > [email protected]
> > >
> > > Wouldn't it be better to use the device name for uinput rather than the address?
> >
> > Hi!
> >
> > Can we access device name in bluez? If yes that would be improvement too!
> >
> > But because more profiles can export input device we still should
> > include profile name (or abbrev) into device name.
>
> Yup, I would be totally ok with doing something like "Your Device Name
> Here (AVCTP)"
That it fine for me.
> >
> > > In `init_uinput` (avctp.c), uinput_create is called with
> > > device_get_address instead of device_get_name. It would probably be
> > > better to use the device name for the uinput and set the device
> > > address as the `uniq` attribute (as it is done for /dev/uhid for HID
> > > devices).
> >
> > And how to set uniq attribute? I'm looking at uinput API, but do not see
> > any way how to set such thing.
> >
> > According to uinput kernel documentation there are two ways how to setup
> > uinput device. New one via UI_DEV_SETUP ioctl and the old one by writing
> > structure via write() syscall.
> >
> > https://www.kernel.org/doc/html/latest/input/uinput.html
> >
> > New way which has to call UI_DEV_SETUP ioctl has following struct
> > uinput_setup members:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/uinput.h#n67
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/input.h#n58
> >
> > u16 bustype;
> > u16 vendor;
> > u16 product;
> > u16 version;
> > char name[UINPUT_MAX_NAME_SIZE];
> > u32 ff_effects_max;
> >
> > Old way how to setup uinput is to call write() systecall with struct
> > uinput_user_dev members, which are:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/uinput.h#n223
> >
> > char name[UINPUT_MAX_NAME_SIZE];
> > u16 bustype;
> > u16 vendor;
> > u16 product;
> > u16 version;
> > u32 ff_effects_max;
> > s32 absmax[ABS_CNT];
> > s32 absmin[ABS_CNT];
> > s32 absfuzz[ABS_CNT];
> > s32 absflat[ABS_CNT];
> >
> > And I do not see any field which could represents that "uniq" attribute.
> > Seems that it can be exported only by kernel input drivers, not uinput
> > userspace drivers...
>
> You are correct -- setting uniq is currently missing in uinput.h.
> (https://github.com/torvalds/linux/blob/6f0d349d922ba44e4348a17a78ea51b7135965b1/include/uapi/linux/uinput.h#L145
> I'm working on a patch to fix that and will send it to
> linux-bluetooth@ mailing list later today.
>
> Setting ATTR{phys} is possible via ioctl(fd, UI_SET_PHYS, phys) and I
> sent a patch to do that yesterday:
> https://lore.kernel.org/linux-bluetooth/20191126160013.BlueZ.1.I95600043ffb5b2b6c4782497ff735ab5d3e37c13@changeid/T/#u
Ok, I did not know that there are additional ioctls for setup. Maybe
this should be put also into documentation.
So not only it is possible, but we have already patch for it. Nice!
>
> >
> > > So in the following, replace ATTR{name} with something like "Some
> > > Company Headphones", ATTR{uniq} = "33:33:33:ff:ff:ff" and ATTR{phys}
> > > with the controller's address.
> > >
> > > $ udevadm info -a -p /sys/devices/virtual/input/input21/
> > > ...
> > > looking at device '/devices/virtual/input/input21':
> > > KERNEL=="input21"
> > > SUBSYSTEM=="input"
> > > DRIVER==""
> > > ATTR{inhibited}=="0"
> > > ATTR{name}=="33:33:33:FF:FF:FF"
> > > ATTR{phys}==""
> > > ATTR{properties}=="0"
> > > ATTR{uniq}==""
> > >
> > > This is what uhid looks like in comparison:
> > >
> > > $ udevadm info -a -p /sys/class/misc/uhid/0005\:046D\:B33B.0002/input/input18/
> > >
> > > looking at device
> > > '/devices/virtual/misc/uhid/0005:046D:B33B.0002/input/input18':
> > > KERNEL=="input17"
> > > SUBSYSTEM=="input"
> > > DRIVER==""
> > > ATTR{inhibited}=="0"
> > > ATTR{name}=="Keyboard K780 Keyboard"
> > > ATTR{phys}=="ab:cd:ef:12:34:56"
> > > ATTR{properties}=="0"
> > > ATTR{uniq}=="33:33:33:33:ff:ff"
> > >
> > >
> > > Abhishek
> >
> > --
> > Pali Rohár
> > [email protected]
--
Pali Rohár
[email protected]
On Wednesday 27 November 2019 18:58:08 Pali Rohár wrote:
> On Wednesday 27 November 2019 09:45:32 Abhishek Pandit-Subedi wrote:
> > On Wed, Nov 27, 2019 at 9:36 AM Pali Rohár <[email protected]> wrote:
> > >
> > > On Wednesday 27 November 2019 09:21:24 Abhishek Pandit-Subedi wrote:
> > > > Wouldn't it be better to use the device name for uinput rather than the address?
> > >
> > > Hi!
> > >
> > > Can we access device name in bluez? If yes that would be improvement too!
> > >
> > > But because more profiles can export input device we still should
> > > include profile name (or abbrev) into device name.
> >
> > Yup, I would be totally ok with doing something like "Your Device Name
> > Here (AVCTP)"
>
> That it fine for me.
Would you prepare patch which change uinput name from address to real
name and add that profile name?
--
Pali Rohár
[email protected]