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

Differentiate multiple devices #13

Open
tomlankhorst opened this issue Sep 18, 2019 · 10 comments
Open

Differentiate multiple devices #13

tomlankhorst opened this issue Sep 18, 2019 · 10 comments
Labels
feature request New feature or request

Comments

@tomlankhorst
Copy link

Is there in the current implementation a way to distinguish input from two or more devices?
On first sight, there seems to be no facility in the event structures of libspnav nor in the raw data from the UNIX sockets that differentiates an event produced by device A or B.

Is there a way to achieve this differentiation?

@jtsiomb
Copy link
Contributor

jtsiomb commented Sep 18, 2019

No, there isn't. I've heard this request a couple of times over the years though, so I'll keep it in mind for the upcoming protocol re-design that I wanted to do for some time now. Leaving this open as a feature request.

@jtsiomb jtsiomb added the feature request New feature or request label Sep 18, 2019
@tomlankhorst
Copy link
Author

Okay, I might submit a (quick) PR these days to realize this for the current protocol.

@ggoretkin-bdai
Copy link

I'm wondering if, instead of changing the protocol, spacenavd could just open a new socket per additional device, using the same protocol per socket. libspnav already has functionality to configure the socket path to use.

It's not ideal from the perspective of hotplugging, but it might be a quicker fix.

@jtsiomb
Copy link
Contributor

jtsiomb commented Jan 19, 2024

That might indeed be a quick and easy way to support multiple devices. It would need some refactoring of how devices are handled internally, but would require no protocol changes. And libspnav could hide most of the details from user apps, making it possibly as simple as setting a global flag about multi-device input before opening the connection, and handling events slightly differently to take device id into account...

@ggoretkin-bdai
Copy link

Digging in a bit more, it looks like the functionality in 7145788 lays a lot of the groundwork for using the existing single-socket approach. If the request protocol (from client to spacenavd) is expanded so that set_client_device can be called, then each client can listen to a specific device. That approach requires some protocol changes, but they should be backwards compatible, requiring an addition to

enum {
	/* per-client settings */
	REQ_SET_NAME = REQ_BASE,/* set client name: Q[0-5] next 24 bytes Q[6] remaining length - R[6] status */
	REQ_SET_SENS,			/* set client sensitivity:	Q[0] float - R[6] status */
	REQ_GET_SENS,			/* get client sensitivity:	R[0] float R[6] status */
	REQ_SET_EVMASK,			/* set event mask: Q[0] mask - R[6] status */
	REQ_GET_EVMASK,			/* get event mask: R[0] mask R[6] status */

@jtsiomb
Copy link
Contributor

jtsiomb commented Jan 20, 2024

If we're adding things to the protocol, I think binding a device to each client (and having the client open multiple connections) feels ackward.

I think I'd rather co-opt, on request by the application to enable multi-device mode, the extremely useless last field of motion events, for sending a device id. So far it was used to transmit a period between events, which nobody ever uses for anything, and I added initially just because the 3dconnexion X11 protocol had it.

The "activate multi-device mode" request can return the number of devices, or a new request can be added for that, as well as a new event type for when devices come and go.

I think all device requests have empty fields which can be used to specify which device the request applies to, so that applications can properly enumerate all devices. And although I'm pretty sure currently all unused fields are set to 0, it's probably prudent to explicitly ignore it if the client has not activated multi-device mode.

In reality since the new protocol was introduced, the biggest difficulty I think is in how to handle multiple devices within the daemon at this point. Beyond the refactoring, there's also the issue of what to do with configuration. Things like device sensitivity, axis and button mappings and so on, are really device-specific, but currently there's just a single global configuration, and changing that would necessarilly complicate the configuration syntax. But I guess one step at a time...

@ggoretkin-bdai
Copy link

If we're adding things to the protocol, I think binding a device to each client (and having the client open multiple connections) feels ackward.

but it seems like this was the intent of

spacenavd/src/event.c

Lines 357 to 360 in d186c5d

client_dev = get_client_device(c);
if(!client_dev || client_dev == dev_ev->dev) {
send_event(&dev_ev->event, c);
}

is there an alternative intent that I'm missing?

@jtsiomb
Copy link
Contributor

jtsiomb commented Jan 20, 2024

No, you're right, that was the intention behind this mechanism, and I guess it's also useful to be able to select a specific device and receive only events from that. But:

  1. This is basically an incomplete stub. client->dev is always null at this point, defaulting to the first device.
  2. This bug report, unless I'm reading it incorrectly, is about receiving events from all devices, and being able to discriminate between them. Implementing that on a per-client device association is possible, but awkward.

@ggoretkin-bdai
Copy link

as well as a new event type for when devices come and go.

I think this is already in place with event_dev / spnav_event_dev

spacenavd/src/dev.c

Lines 212 to 220 in a2d121a

/* new USB device added, send device change event */
ev.dev.type = EVENT_DEV;
ev.dev.op = DEV_ADD;
ev.dev.id = dev->id;
ev.dev.devtype = dev->type;
ev.dev.usbid[0] = dev->usbid[0];
ev.dev.usbid[1] = dev->usbid[1];
broadcast_event(&ev);
break;

@jtsiomb
Copy link
Contributor

jtsiomb commented Jan 22, 2024

Nice! I forgot I had done that. Unfortunately I also forgot to add the various device operations (like DEV_ADD) in the protocol header. So that part is covered, and the omissions are easily mended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants