On Wed, Nov 27, 2019 at 9:36 AM Pali Rohár <pali.rohar@xxxxxxxxx> 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 <pali.rohar@xxxxxxxxx> 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 <pali.rohar@xxxxxxxxx> 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 > > > pali.rohar@xxxxxxxxx > > > > 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 > pali.rohar@xxxxxxxxx
