search

monome / norns

norns is many sound instruments.
http://monome.org
GNU General Public License v3.0
630 stars 145 forks source link

HID devices double-freed #666

Closed catfact closed 5 years ago

catfact commented 5 years ago

HID lua layer is confusing at best and almost certainly has bugs.

for example, it appears that some devices fail to be removed on unplug, others get attempted double-frees.

as reported on this PR https://github.com/monome/norns/pull/662

okyeron commented 5 years ago

FWIW - I may have caused the "fail to be removed" or "double-free" problems in my rewrite. ~I'm not entirely sure (yet)~.

EDIT - Reverted my install to the current norns/master and I can confirm the double free error on device disconnect happens with the existing code.

okyeron commented 5 years ago

I think I've fixed the outstanding issues with HID.

https://github.com/monome/norns/pull/662 updated today.

tehn commented 5 years ago

great! are you now seeing the correct behavior in your testing with hardware?

okyeron commented 5 years ago

Yes. The previously leftover device connection from my keyboard (which connects as 2 devices) gets removed now.

I'll do some additional testing later today with the Wacom tablet, mouse and anything else i can scrounge up. :)

okyeron commented 5 years ago

Trying to add menu device selection like with grid/midi...

Devices like the Wacom tablet and my keyboard enumerate as multiple items in hid.list - so what's the best way to select those in menu? Currently I see 3 items for the Wacom and the select list only holds 4 things

hid.list:
1   Intuos5 touch M
2   Intuos5 touch M
3   Intuos5 touch M
4   USB Optical Mouse
5   iMate, USB To ADB Adaptor
6   iMate, USB To ADB Adaptor

EDIT - what I'm trying to say is that the selection list does not scroll for some reason.

okyeron commented 5 years ago

menu.lua m.redraw[pDEVICES] would need some re-jiggering to get scrolling to work for a hid listing.

So far I've not be able to figure out a good way to make scrolling work for that list.

It'd be nice if there was a general function for building a scrolling list (as it seems all of those in menu are pretty specialized)

artfwo commented 5 years ago

@okyeron there are only 4 usb ports, so in theory scrolling shouldn't be required :) i'd try debugging why some devices have multiple entries first, inspect what udev and matron know about plugged devices.

okyeron commented 5 years ago

@artfwo right - however, some hid devices enumerate as multiple /dev/input/event items. For example my Apple extended II keyboard has two event items - one for keyboard and another for mouse (since that keyboard acts as a hub for mouse connection).

As a better example, the Wacom intuos tablet shows up as 3 devices. (looking at hid.devices) Each of those 3 devices has different codes.

If I print those from lua - digging into the hid.devices.codes table - I get the following

Dev 1 Codes:
328 BTN_TOOL_QUINTTAP
330 BTN_TOUCH
325 BTN_TOOL_FINGER
333 BTN_TOOL_DOUBLETAP
334 BTN_TOOL_TRIPLETAP
335 BTN_TOOL_QUADTAP

Dev 2 Codes:
256 BTN_0
257 BTN_1
258 BTN_2
259 BTN_3
260 BTN_4
261 BTN_5
262 BTN_6
263 BTN_7
264 BTN_8
331 BTN_STYLUS

Dev 3 Codes:
320 BTN_TOOL_PEN
321 BTN_TOOL_RUBBER
322 BTN_TOOL_BRUSH
275 BTN_SIDE
276 BTN_EXTRA
326 BTN_TOOL_MOUSE
327 BTN_TOOL_LENS
323 BTN_TOOL_PENCIL
324 BTN_TOOL_AIRBRUSH
330 BTN_TOUCH
331 BTN_STYLUS
332 BTN_STYLUS2
272 BTN_LEFT
273 BTN_RIGHT
274 BTN_MIDDLE

What I'm not sure about is if all of those codes come through with the hid.connect(1) connection as I have it right now. I think they do - so perhaps this is a moot point.

tehn commented 5 years ago

i did in fact just add listselect.lua to one of the new branches. i can use that for the device selection--- thanks for the suggestion. (i'll add it later, this PR can just be about functionality).

will be curious to hear how your testing goes with codes coming from each device

okyeron commented 5 years ago

@tehn functionality seems good right now. my super basic test script gives me a big stream of event data - for example, the Wacom constantly streams x,y coordinates when the pen is above the surface, but not touching. Mouse gets x,y + buttons + scrollwheel. 

local keyb = hid.connect(1) -- connection by vport number not device id
function keyb.event(typ, code, val)
    print("hid.event ", typ, code, val)
end

I've not yet figured out parsing the data, but it's not too bad once you know what codes the device uses - which I've just been getting from the hid.devices table tab.print(hid.devices[5].codes) or similar.

so maybe a get-device-info function is needed?

tehn commented 5 years ago

so maybe a get-device-info function is needed?

not sure what exactly this would be?

related: https://github.com/monome/norns/blob/master/lua/hid.lua#L155

okyeron commented 5 years ago

Oh - I probably need to try Hid:print() and see if that works or gives good info. (Havent look at that at all yet)

I was thinking something like a get-info script/function that shows whats available for that device so scripters would know what codes are there, etc.

I don't have much of an idea on how to move that FIXME stuff into matron. :(

catfact commented 5 years ago

that FIXME is just about tidiness / DRYness really. the amount of data coming into lua on device discovery is maybe a bit over the top; but hey, it seems to work. (very gratifying that you can see all that stuff from the wacom.)

a further layer of abstraction in lua would serve the same function as hiding all the event code table stuff in the C side. (IMHO.)