Skip to content

Enable Hidraw interface on spotlight bluetooth connection - #140

Merged
jahnf merged 1 commit into
gbin:feature/spotlight-hidraw-functionalityfrom
mayanksuman:enable_hidraw_on_bluetooth_connection
Jul 19, 2021
Merged

Enable Hidraw interface on spotlight bluetooth connection#140
jahnf merged 1 commit into
gbin:feature/spotlight-hidraw-functionalityfrom
mayanksuman:enable_hidraw_on_bluetooth_connection

Conversation

@mayanksuman

@mayanksuman mayanksuman commented Jul 16, 2021

Copy link
Copy Markdown
Contributor

This commit enables hidraw interface on logitech spotlight connected via
bluetooth. Three major changes are

  1. DeviceId (in device.h) now include the information about the bus on
    which spotlight is connected (USB/Bluetooth).
  2. sendData function (device.cc) for hidraw on bluetooth modifies the data
    before sending it. The bluetooth hid need the data in 20 byte long
    packages; smaller packet of 7 byte length is not allowed on bluetooth
    connection. More details for this conversion is provided in the
    function defintion as comment.
  3. The projecteur now initialize HID device correctly (Get rid of any
    device configuration by other programs). The projecteur also pings
    the device and check the the HID++ version supported by the device.

With this code, device can vibrate (and support other functionality as
being worked in #136 ) even if it is connected on bluetooth.

Note: To connect logitech spotlight using bluetooth, press top button
and the last button till the led light starting flashing. The spotlight
device can now be paired with computer.

@mayanksuman
mayanksuman force-pushed the enable_hidraw_on_bluetooth_connection branch from 67eea21 to 904d9fa Compare July 16, 2021 15:59
@mayanksuman

mayanksuman commented Jul 16, 2021

Copy link
Copy Markdown
Contributor Author

Important Note: The spotlight device is designed for only one active connection at a time. The Projecteur seem to not work when device is connected by both means (USB and Bluetooth). However, to correct it press any key on spotlight device. The bluetooth connection will automatically disconnect and Projecteur will be able to work again.

@mayanksuman
mayanksuman force-pushed the enable_hidraw_on_bluetooth_connection branch 6 times, most recently from 3dc4282 to 61f1426 Compare July 17, 2021 06:49
@jahnf

jahnf commented Jul 17, 2021

Copy link
Copy Markdown
Collaborator

Hi @mayanksuman , thank you for your work 👍 - I am currently traveling this weekend - but will review and integrate via feature branch in the next days.

@mayanksuman
mayanksuman force-pushed the enable_hidraw_on_bluetooth_connection branch 3 times, most recently from e9efb50 to c4a144c Compare July 17, 2021 20:42
@jahnf
jahnf changed the base branch from develop to feature/spotlight-hidraw-functionality July 18, 2021 07:27

@jahnf jahnf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mayanksuman, I have some remarks. Most are about the sleeps.
If there needs to be a delay between data send to the device, we need a better way - e.g. schedule it with a timer instead of just blocking the main thread.

Also: when pinging the device - why not make sure we get a reply?

Comment thread src/device-vibration.cc
Comment thread src/device.cc Outdated
Comment thread src/device.cc Outdated
Comment thread src/device.cc Outdated
Comment thread src/device.h Outdated
Comment thread src/device.h Outdated
Comment thread src/device.h Outdated
@mayanksuman
mayanksuman force-pushed the enable_hidraw_on_bluetooth_connection branch from c4a144c to b6bf092 Compare July 18, 2021 08:35
@mayanksuman

mayanksuman commented Jul 18, 2021

Copy link
Copy Markdown
Contributor Author

Hi @jahnf , all the comments have taken care of. I have used QTimer instead of nanosleep.

We are checking the response of ping in async manner. We are not putting a timer with timeout for this case. Rather, the response to all the HID commands are handled in onHIDDataAvailable function in src/spotlight.cc, whenever it is available.

It is not necessary that device send information only on request, sometime like connecting device for charging or other cases, it sends the information on its own. Hence, I thought that instead of checking the response to each HID command, let the HID response come and process it when it is available.

@mayanksuman
mayanksuman force-pushed the enable_hidraw_on_bluetooth_connection branch 2 times, most recently from 71945e2 to e67513e Compare July 18, 2021 17:33
This commit enables hidraw interface on logitech spotlight connected via
bluetooth. Three major changes are
1. DeviceId (in device.h) now include the information about the bus on
   which spotlight is connected (USB/Bluetooth).
2. sendData function (device.cc) for hidraw on bluetooth modifies the data
   before sending it. The bluetooth hid need the data in 20 byte long
   packages; smaller packet of 7 byte length is not allowed on bluetooth
   connection. More details for this conversion is provided in the
   function definition as comment.
3. The projecteur now initialize HID device correctly (Get rid of any
   device configuration by other programs). The projecteur also pings
   the device and check the the HID++ version supported by the device.

Note: To connect logitech spotlight using bluetooth, press top button
and the last button till the led light starting flashing. The spotlight
device can now be paired with computer.
@mayanksuman
mayanksuman force-pushed the enable_hidraw_on_bluetooth_connection branch from e67513e to cf79056 Compare July 18, 2021 17:53
@jahnf jahnf added this to the v1.0 milestone Jul 19, 2021
@jahnf
jahnf merged commit b1cd1f6 into gbin:feature/spotlight-hidraw-functionality Jul 19, 2021
@jahnf

jahnf commented Jul 19, 2021

Copy link
Copy Markdown
Collaborator

Hi @mayanksuman

What Linux-Distro are you running?
I encounter two problems with this on my Ubuntu 20.04 machines when using bluetooth with the Spotlight device.

  • The hidraw sub-device is not readable by the user - so only running as root will correctly establish a hidraw sub-device
    -> This might just be a solvable with small changes the udev rules file - I will investigate more into this

  • But more importantly: Even when I run Projecteur as root - everytime I try to send the vibrate command I get a "The device is not active. Activate it by pressing any button on device."

[2021-07-19T08:02:28.567][dbg][projecteur.HID] Write 1101001d00005d00000000000000000000000000 to /dev/hidraw3
[2021-07-19T08:02:28.567][dbg][projecteur.HID] Write 1101051d00000000000000000000000000000000 to /dev/hidraw3
[2021-07-19T08:02:29.108][dbg][projecteur.HID] Received 1101051d00000000000000000000000000000000 from /dev/hidraw3
[2021-07-19T08:02:48.765][inf][projecteur.HID] The device is not active. Activate it by pressing any button on device.
[2021-07-19T08:02:53.565][inf][projecteur.HID] The device is not active. Activate it by pressing any button on device.
[2021-07-19T08:02:56.415][inf][projecteur.HID] The device is not active. Activate it by pressing any button on device.
[2021-07-19T08:03:08.662][inf][projecteur.HID] The device is not active. Activate it by pressing any button on device.

As you collected a lot of experience with the device, what could be the reason for this?

Just for information, the Bluetooth device on my machine detected by Projecteur looks like this:

Projecteur 1.0-alpha.33; device scan

 * Found 1 supported devices. (0 readable, 0 writable)

 +++ name:     'SPOTLIGHT'
     userName: 'Logitech Spotlight (Bluetooth)'
     vendorId:  046d
     productId: b503
     phys:      f8:e4:e3:01:27:e5
     busType:   Bluetooth
     devices:   /dev/input/event24, /dev/input/event23, /dev/input/event22, /dev/hidraw3
     readable:  false
     writable:  false

@mayanksuman

mayanksuman commented Jul 19, 2021

Copy link
Copy Markdown
Contributor Author

What Linux-Distro are you running?

I am using Debian Testing on my main machine.

In your device scan, some sub-device (most probably /dev/hidraw3) is not readable or writable. So, even if the device is sending a connection active packet ({0x11, 0x01, 0x04, ...}), the software (Projecteur) is not able to receive it and hence reports that the device is not active.

I think, this is a issue with udev rule. One more point, are you running a virtual machine?

I have not tested bluetooth connection on virtual machine, as my machine do not provide mean to pass bluetooth device to virtual machine.

@jahnf

jahnf commented Jul 19, 2021

Copy link
Copy Markdown
Collaborator

Nope not a virtual machine and all the sub-devices are readable when running as root.
I have the issue even when running as root. As you see in the log, data gets written to hidraw3 and also received, but
when sending the vibrate command it results in this:

[2021-07-19T08:02:28.567][dbg][projecteur.HID] Write 1101001d00005d00000000000000000000000000 to /dev/hidraw3
[2021-07-19T08:02:28.567][dbg][projecteur.HID] Write 1101051d00000000000000000000000000000000 to /dev/hidraw3
[2021-07-19T08:02:29.108][dbg][projecteur.HID] Received 1101051d00000000000000000000000000000000 from /dev/hidraw3
[2021-07-19T08:02:48.765][inf][projecteur.HID] The device is not active. Activate it by pressing any button on device.
[2021-07-19T08:02:53.565][inf][projecteur.HID] The device is not active. Activate it by pressing any button on device.

Device scan as root:

Projecteur 1.0-alpha.33; device scan

 * Found 1 supported devices. (1 readable, 1 writable)

 +++ name:     'SPOTLIGHT'
     userName: 'Logitech Spotlight (Bluetooth)'
     vendorId:  046d
     productId: b503
     phys:      f8:e4:e3:01:27:e5
     busType:   Bluetooth
     devices:   /dev/input/event22, /dev/input/event23, /dev/input/event24, /dev/hidraw3
     readable:  true
     writable:  true

Edit: added addtional udev rules, so that as a user all sub-devices are now readable - issue remains the same when running as a regular user. - Don't have too much time now to investigate further, but any idea that you have is appreciated - will investigate more later this week.

@mayanksuman

mayanksuman commented Jul 19, 2021

Copy link
Copy Markdown
Contributor Author

Hi @jahnf ,

My mistake. Device activation packet is only sent in case of USB connection. If device is connected on Bluetooth it should be considered online all the time.

Please change the isOnline function definition in device.h to

bool isOnline() { return (m_details.busType == BusType::Bluetooth ||
                            m_details.hidProtocolVer > 0); };

You can directly merge #142 in your feature branch with this fix.

@jahnf

jahnf commented Jul 19, 2021

Copy link
Copy Markdown
Collaborator

Thanks, works now - will review the battery PR within the next days.

@mayanksuman

Copy link
Copy Markdown
Contributor Author

Ok ...
Should I rebase battery PR to your feature/spotlight-hidraw-functionality branch?

@jahnf

jahnf commented Jul 19, 2021

Copy link
Copy Markdown
Collaborator

Yes that would be helpful - I'd like to do tests on the feature branch for multiple systems before finally merging to develop.

@mayanksuman

mayanksuman commented Jul 19, 2021

Copy link
Copy Markdown
Contributor Author

Ok ... I will rebase and push on that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants