Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BLE HID OUT report handling seems not to be implemented. #207

Open
ThomasAtBBTF opened this issue Nov 5, 2024 · 5 comments
Open

BLE HID OUT report handling seems not to be implemented. #207

ThomasAtBBTF opened this issue Nov 5, 2024 · 5 comments

Comments

@ThomasAtBBTF
Copy link

ThomasAtBBTF commented Nov 5, 2024

For my project to create a BLE HID Braille device I tried to read in the BLE HID keyboard code the LED Status and got an error.

Here is the CP 9.2 CircuitPython code:

import sys
import time
import board
from digitalio import DigitalInOut, Direction, Pull
import adafruit_ble
from adafruit_ble.advertising import Advertisement
from adafruit_ble.advertising.standard import ProvideServicesAdvertisement
from adafruit_ble.services.standard.hid import DEFAULT_HID_DESCRIPTOR, HIDService
from adafruit_ble.services.standard.device_info import DeviceInfoService
from adafruit_hid.keyboard import Keyboard
from adafruit_hid.keycode import Keycode

print("starting HIDService ")
hid = HIDService(DEFAULT_HID_DESCRIPTOR)
device_info = DeviceInfoService(software_revision=adafruit_ble.__version__, manufacturer="BBTF GmbH")
advertisement = ProvideServicesAdvertisement(hid)
advertisement.appearance = 961  # HID Keyboard
scan_response = Advertisement()
scan_response.complete_name = "MyPyKeyboard"

print("starting BLERadio")
ble = adafruit_ble.BLERadio()

print("starting Keyboard")
k = Keyboard(hid.devices)
ble_connected = False
ble_advertising = False

print("start Advertising")
if not ble.connected:
    ble.start_advertising(advertisement, scan_response)
    ble_advertising = True


def k_send(keycode):
    try:
        print("sending:", keycode)
        k.send(keycode)
        print("sent")
    except Exception as e:
        print("Exception in k_send:", e)
    return


loop_counter = 0
io0 = DigitalInOut(board.IO0)
io0.direction = Direction.INPUT
io0.pull = Pull.UP
last_io0_value = io0.value

print("Main Loop")
while True:
    loop_counter += 1
    if loop_counter > 20:
        if ble_advertising:
            print("Advertising")
        if ble_connected:
            print("Connected")
        loop_counter = 0
    if ble.connected:
        if not ble_connected:
            ble_connected = True
            print("Connected to BLERadio")
        if ble_advertising:
            ble_advertising = False
            # ble.stop_advertising()
            print("Stopping Advertising")
        if last_io0_value != io0.value:
            last_io0_value = io0.value
            if not last_io0_value:
                k_send(Keycode.A)
            print("Type(k) :", type(k))
            print("dir(k) :", dir(k ))
            print("type(k._keyboard_device) :", type(k._keyboard_device))
            print("Type(k.led_status)", type(k.led_status))
    else:
        if ble_connected:
            ble_connected = False
        if not ble_advertising:
            print("Not connected to BLERadio")
            ble.start_advertising(advertisement, scan_response)
            ble_advertising = True

I am using (for test purposes) the "boot" button of the ESP32 board to send the character "a" and after sending the character I am testing the led_status.

In order to understand the objects involved I am printing the Type and the members.

The function / property k.led_status tries to get "self._keyboard_device.get_last_received_report()"
which fails with a runtime error.

Here is my log from the test:

code.py output:
starting HIDService
starting BLERadio
starting Keyboard
devices: <class 'list'> [<ReportOut object at 0x3c19fc90>, <ReportIn object at 0x3c19fca0>, <ReportIn object at 0x3c1a0200>, <ReportIn object at 0x3c1a0480>]
looking for 0x1 0x6
device: <ReportOut object at 0x3c19fc90> <class 'ReportOut'> 0x1 0x6
device: <ReportIn object at 0x3c19fca0> <class 'ReportIn'> 0x1 0x6
found!
start Advertising
Main Loop
Advertising
Connected to BLERadio
Stopping Advertising
Connected
Connected
Connected
Connected
Connected
Connected
sending: 4
sent
Type(k) : <class 'Keyboard'>
dir(k) : ['__class__', '__init__', '__module__', '__qualname__', 'send', '__dict__', 'press', 'release', 'release_all', 'report', '_keyboard_device', 'led_status', 'LED_NUM_LOCK', 'LED_CAPS_LOCK', 'LED_SCROLL_LOCK', 'LED_COMPOSE', 'report_modifier', 'report_keys', '_led_status', '_add_keycode_to_report', '_remove_keycode_from_report', 'led_on']
type(k._keyboard_device) : <class 'ReportIn'>
Traceback (most recent call last):
  File "code.py", line 75, in <module>
  File "/lib/adafruit_hid/keyboard.py", line 180, in led_status
AttributeError: 'ReportIn' object has no attribute 'get_last_received_report'

Code done running.

I am using a FeatherS3 from UM

PS: I modified "find_device" to see what it does while finding the requested device which explains the debug output after
"starting Keyboard" and before "start Advertising".

Like so:

def find_device(
    devices: Sequence[object],
    *,
    usage_page: int,
    usage: int,
    timeout: int = None,
) -> object:
    """Search through the provided sequence of devices to find the one with the matching
    usage_page and usage.

    :param timeout: Time in seconds to wait for USB to become ready before timing out.
    Defaults to None to wait indefinitely.
    Ignored if device is not a `usb_hid.Device`; it might be BLE, for instance."""

    print("devices:", type(devices), devices)
    if hasattr(devices, "send_report"):
        print("has send_report")
        devices = [devices]  # type: ignore
    device = None
    print("looking for", hex(usage_page), hex(usage))
    for dev in devices:
        print("device:", dev, type(dev), hex(dev.usage_page), hex(dev.usage))
        if (
            dev.usage_page == usage_page
            and dev.usage == usage
            and hasattr(dev, "send_report")
        ):
            print("found!")
            device = dev
            break
    if device is None:
        raise ValueError("Could not find matching HID device.")

    # Wait for USB to be connected only if this is a usb_hid.Device.
    if Device and isinstance(device, Device):
        if supervisor is None:
            # Blinka doesn't have supervisor (see issue Adafruit_Blinka#711), so wait
            # one second for USB to become ready
            time.sleep(1.0)
        elif timeout is None:
            # default behavior: wait indefinitely for USB to become ready
            while not supervisor.runtime.usb_connected:
                time.sleep(1.0)
        else:
            # wait up to timeout seconds for USB to become ready
            for _ in range(timeout):
                if supervisor.runtime.usb_connected:
                    return device
                time.sleep(1.0)
            raise OSError("Failed to initialize HID device. Is USB connected?")

    return device
@ThomasAtBBTF
Copy link
Author

It seems to me, that the find_device is "only" looking for a device with the desired usage_page and usage AND an attribute "send_report".

But for a device of type ReportOut this will never be the case.

I think: A Device should be / have a collection of several ReportOut and ReportIn objects....

@ThomasAtBBTF
Copy link
Author

ThomasAtBBTF commented Nov 6, 2024

With some small modifications in adafruit_hit_init_.py in the function "def find_device"
I am now storing the ReportIn and the ReportOut objects in a list as the device which find_device returns.
In Keyboard.py I am not calling "self._keyboard_device" anymore directly but selecting the "correct Report object"
for calling "reportin.send_report" and when trying to get the LED status of the keyboard I am calling "reportout.report". (which is a bytearray).

By doing so, I continue to be able to send reports as well as obtaining the led_status.
The led_status represented by "reportout.report" correctly reflects the values of caps lock, num_lock and scroll lock when these are changed.

This leads me to believe that the underlying C implementation is working fine! (at least for a BLE HID keyboard)

Next I will investigate how the underlying C implementation is handling descriptors which are asking for multiple ReportIn and ReportOut objects.

@dhalbert
Copy link
Collaborator

dhalbert commented Nov 6, 2024

There is no underlying C implementation specifically for BLE ReportIn and ReportOut objects. They are implemented in Python. They use standard BLE-related classes (some of which are in C).

The current structure of ReportIn and ReportOut acting directly as devices is wrong. There should be a Device class that provides multiple reports, both IN and OUT. I can work on that, or you can, if this is clear to you. We would use usb_hid.Device as a model. ReportIn and ReportOut don't need to be user-visible classes.

@ThomasAtBBTF
Copy link
Author

ThomasAtBBTF commented Nov 6, 2024

@dhalbert
I can work on it, but before doing so, we need to clarify the direction we want to take. -I think-

How I see it:

  1. The function find_device has to change and return the list of ReportIn and ReportOut objects which are "making up" the device.
    This would give the actual Device class the possibility to assign ReportXXX objects to nicely "nameable" members inside the Device class.

1a. maybe the find_device function shall not be changed to provide/maintain back ward compatibility with older clients.
1.b. a new function "find_device_reports" shall be implemented to provide the new functionality.

  1. The Device classes shall be changed to have these nicely named internal members which can provide the desired functionality to public member functions / properties of the Devices.

  2. Step by Step the most important Devices shall be adapted (which ones?) to the new scheme.

  3. Checking where and how the ReportOut data can be queued.

  4. Creating sample code how a "new" BLE HID Device can/shall be created.
    Maybe the BrailleDisplay Device class? But I have dougths here, because it seems to be the most complex HID device as far as I see from browsing the HUT_x.PDF documentation. It has multiple "IN reports" and at least 2 (if not more) "OUT reports".

@dhalbert
Copy link
Collaborator

I am suggesting making a class with an API more or less identical to https://docs.circuitpython.org/en/latest/shared-bindings/usb_hid/index.html#usb_hid.Device. That is implemented in C: https://github.com/adafruit/circuitpython/blob/main/shared-module/usb_hid/Device.c. The BLE class would not be implemented in C.

The ReportIn and ReportOut classes would disappear from the user's viewpoint. They might be used internally or might not.

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

No branches or pull requests

2 participants