Skip to content

Conversation

@qrsikno2
Copy link
Contributor

There is the background of this api:
#249
#250

return device.vendor_id == 0x1233
end,

report_descriptor = {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you clarify from where these values come from? what's the original driver?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I write it to run a imaginary driver. That's because a set of devices may have their own quirks(maybe) which make it very difficult to cope with diffierent situation. But The draft needs a full example for diffierent apis i think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if needed, i will redraft an example to "port" a driver.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would start by "porting" a driver using the proposed API.. so we can have a better sense..

local function dummy_submitx(value)
-- pass
end
local function dummy_submity(value)
Copy link
Contributor

Choose a reason for hiding this comment

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

please, mind \n's.. observer the style of other examples..

local function dummy_submitx(value)
-- pass
end
local function dummy_submity(value)
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the purpose of these two funcs?

local function dummy_submity(value)
-- pass
end
local slow_mouse_driver = {
Copy link
Contributor

Choose a reason for hiding this comment

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

please, add readme entry for describing this API and every field in this table..

-- example of a slow mouse driver
local hid = require("luahid")

local fixed_xiaomi_report_descriptor = {
Copy link
Contributor

@lneto lneto May 25, 2025

Choose a reason for hiding this comment

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

please, add a comment with a link to reference kernel code of the original driver

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

how do you plan to parse and use such values in your binding? would it be an actual binding or will it ending up being tied to this particular driver?

Copy link
Contributor Author

@qrsikno2 qrsikno2 May 26, 2025

Choose a reason for hiding this comment

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

yes, the link is right.
what is the difference between "actual binding" and "being tied to this particular driver"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please, add a comment with a link to reference kernel code of the original driver请添加一条评论,并附上引用原始驱动内核代码的链接

done

{ vendor_id = 0x2717, product_id = 0x5014 }, -- copied from kernel src
},

report_descriptor = fixed_xiaomi_report_descriptor
Copy link
Contributor

Choose a reason for hiding this comment

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

that's it? don't you need any callback? just this descriptor?

Copy link
Contributor

Choose a reason for hiding this comment

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

you probably should support callbacks on your API, instead of just assuming such table.. devices drivers should use such callbacks to do their own work.. e.g., https://elixir.bootlin.com/linux/v6.14.7/source/drivers/hid/hid-xiaomi.c#L64

Copy link
Contributor

Choose a reason for hiding this comment

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

btw, that's one reason this example (xiaome mouse) is not that good.. because it's very limited.. if you see, https://elixir.bootlin.com/linux/v6.14.7/source/include/linux/hid.h#L811, we have many other callbacks to be defined that you must give support in your API..

Copy link
Member

Choose a reason for hiding this comment

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

@qrsikno2 , I agree with Lourival here. Using callbacks will make the luahid library "generic". By generic we mean:

  1. You cannot directly call the kernel C functions from lua scripts (E.g: xiaomi_mouse.lua)
  2. So you create a library luahid that will provide an interface (API) to the lua scripts.
  3. The luahid must allow the scripts to register their own callbacks, in this sense, this library becomes generic. The use of callbacks will allow you to register your own functions for the xiaomi mouse or apple keyboard. This is how even the kernel works. The hid_driver is a struct with function pointers so that any driver can use this struct and define their own drivers, hence the hid API is generic.

So xiaomi_mouse.lua calls the luahid functions, which call the Kernel C functions.

Copy link
Contributor Author

@qrsikno2 qrsikno2 May 26, 2025

Choose a reason for hiding this comment

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

I think there is no need to have callbacks in this scene. that is because the hid-xiaomi is only going to fix special device's report descriptor, so you just need to provide the vendor_id+product_id+descriptor. and this makes the example very clear and simple

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think i would show (callbacks apis) in the other example, but xiaomi is the simplest one, so i ported it firstly.
I was very consider about that i cannot show all the apis just by true examples, just because their usage is fragmented

Copy link
Contributor Author

@qrsikno2 qrsikno2 May 26, 2025

Choose a reason for hiding this comment

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

that's it? don't you need any callback? just this descriptor?

yes, just because the default functions have been implemented by the kernel.
and the device's usage(about ranges, buttons and etc) is defined by the descriptor.

that's one reason this example (xiaome mouse) is not that good..

should i change the code to port another driver or just add(not delete but keep the xiaomi mouse as a simplest example) another example for the "another" more complex mouse driver ?

Copy link
Member

Choose a reason for hiding this comment

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

By callback we mean this:
The hid-xiaomi.c (lines 85-90), the descriptors are set by a function called xiaomi_report_fixup which is a callback function, that will be called by the kernel. But in the example you have directly exported the descriptor table.

static struct hid_driver xiaomi_driver = {
	.name = "xiaomi",
	.id_table = xiaomi_devices,
	.report_fixup = xiaomi_report_fixup,
};

This is fine for a single implementation. But since luahid is a library, it needs to handle all types of HID devices, so it's better to handle this in a function. In that function you can just return the descriptor table. Something like this:

local function xiaomi_report_fixup()
    return fixed_xiaomi_report_descriptor
end

In the driver table, you can set it:

local xiaomi_mouse_driver = {
	name = "luahid_xiaomi_mouse_driver",
	...
    report_fixup = xiaomi_report_fixup
}

Copy link
Member

Choose a reason for hiding this comment

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

So your luahid will use this table to then use your functions and tables to pass on to the kernel in a general way for every program.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, changed in new commit =)

local xiaomi_mouse_driver = {
name = "luahid_xiaomi_mouse_driver",
match_list = {
{ vendor_id = 0x2717, product_id = 0x5014 }, -- copied from kernel src
Copy link
Contributor

Choose a reason for hiding this comment

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

please, add a link from the original source

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

@qrsikno2 qrsikno2 May 26, 2025

Choose a reason for hiding this comment

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

I think i would change the pattern of match_list just because some devices need to given a bus_type and device_type.
Is that ok for my apis?

Copy link
Member

Choose a reason for hiding this comment

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

As my previous comment, the match_list is of type struct hid_device_id in the kernel right ?

struct hid_device_id {
	__u16 bus;
	__u16 group;
	__u32 vendor;
	__u32 product;
	kernel_ulong_t driver_data;
};

So this can just be a device id table,

local xiaomi_id = {
    bus = 0x123,
    group = 0x2345, -- sample values
    vendor = 0x11,
    procuct = 0x01
}

-- under the xiaomi driver, you can use this
local xiaomi_mouse_driver = {
	name = "luahid_xiaomi_mouse_driver",
        ....
        id_table = xiaomi_id
}
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes quite right. i think the table can be write into 'plural' form:

local xiaomi_id1 = {
    bus = 0x123,
    group = 0x2345, -- sample values
    vendor = 0x11,
    procuct = 0x01
}
local xiaomi_id2 = {
    bus = 0x123,
    group = 0x2345, -- sample values
    vendor = 0x11,
    procuct = 0x02
}

-- under the xiaomi driver, you can use this
local xiaomi_mouse_driver = {
	name = "luahid_xiaomi_mouse_driver",
        ....
        id_table = {xiaomi_id1, xiaomi_id2},
        -- or id_table = {xiaomi_id1}, embraced with '{}' maybe better.
}
...

name = "luahid_simple_generic_mouse_driver",
}

hid.register(mouse_driver)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the point of this script.. can you explain it or remove it from the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@lneto lneto force-pushed the master branch 2 times, most recently from 17ac113 to 925bfd9 Compare July 14, 2025 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants