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

basic stuff for single, and double left click mouse support #51

Closed
wants to merge 3 commits into from

Conversation

jdavidthomson
Copy link

This is a first go of basic left mouse support with the driver. By tapping on the side of the XReal Air, you can initiate a "tap" event. This was already existing in wheaney's code.

The updates with this PR move the following down in index and add a single left mouse click, and a double left mouse click:
Original:
#MT_RECENTER_SCREEN 2
#define MT_RESET_CALIBRATION 3

PR submission:
#define MT_SINGLE_CLICK 1
#define MT_DOUBLE_CLICK 2
#define MT_RECENTER_SCREEN 3
#define MT_RESET_CALIBRATION 4

There are additional changes to be made, and will come later, but this should help with basics.

void doubletap(uint32_t timestamp_ms, imu_quat_type quat, imu_euler_type velocities, bool ipc_enabled,
bool imu_calibrated, ipc_values_type *ipc_values) {
singletap(timestamp_ms, quat, velocities, ipc_enabled, imu_calibrated, ipc_values);
singletap(timestamp_ms, quat, velocities, ipc_enabled, imu_calibrated, ipc_values);
Copy link
Owner

Choose a reason for hiding this comment

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

I think there should be a delay here? Just to make it seem a little more human in case some apps care. Like most OS's allow you to configure expectations around double-click.

@@ -225,6 +225,19 @@ void deinit_outputs() {
evdev = NULL;
}
}
void singletap(uint32_t timestamp_ms, imu_quat_type quat, imu_euler_type velocities, bool ipc_enabled,
bool imu_calibrated, ipc_values_type *ipc_values) {
libevdev_uinput_write_event(uinput, EV_KEY, BTN_LEFT,1);
Copy link
Owner

Choose a reason for hiding this comment

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

We should be doing something like the checks we have below, since a double-click only makes sense in these contexts (VR-Lite mouse mode specifically, should probably ignore for joystick mode):

    if (uinput) {
        if (is_joystick_mode(context.config)) {
            ...
        } else if (is_mouse_mode(context.config)) {

#define MT_RESET_CALIBRATION 3
#define MT_SINGLE_CLICK 1
#define MT_DOUBLE_CLICK 2
#define MT_RECENTER_SCREEN 3
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm... I think this is problematic, though I'm not sure the solution. I'd like to keep recenter/recalibrate at 2 and 3 taps, respectively, for a couple reasons: multi-tap is difficult to get right for some, so going beyond double-tap for something you may use a lot can add a bit of pain, and all my documentation, UI, and videos talk about double-tap for recenter.

The other issue I see with this is having something use single-tap. False-positives happen sometimes during head movements, so I feel hesitant adding something that triggers after just one tap.

@@ -7,7 +7,7 @@
#include <stdlib.h>
#include <stdio.h>

#define MT_BUFFER_MS 25
#define MT_BUFFER_MS 15
Copy link
Owner

Choose a reason for hiding this comment

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

I haven't played with these changes so I'm not sure if they make things better or worse, and this also tends to be a subjective experience, so the current settings work well for some but not others, etc... I would want to have this vetted through a decent sized group. Maybe it can go in a PR separate from this one if you meant to include it and think it's better than the current settings?

bool imu_calibrated, ipc_values_type *ipc_values) {
singletap(timestamp_ms, quat, velocities, ipc_enabled, imu_calibrated, ipc_values);
singletap(timestamp_ms, quat, velocities, ipc_enabled, imu_calibrated, ipc_values);
}

void handle_imu_update(uint32_t timestamp_ms, imu_quat_type quat, imu_euler_type velocities, bool ipc_enabled,
Copy link
Owner

Choose a reason for hiding this comment

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

If we move forward with this, I'm thinking we could actually just modify this signature with a param like int tapsDetected and then you'd be able to move the single/double tap stuff into the is_mouse_mode section that's already here.

@wheaney
Copy link
Owner

wheaney commented Jul 18, 2024

Hey, let's get this closed out. I think we should either:

  • Put this change behind a configuration so it's enabled just for people that want to use it. Then we could have single-click be a double-tap and double-click be a triple-tap or something to reduce false-positives that would come with supporting a single-tap.
  • Just close the PR without merging

@jdavidthomson
Copy link
Author

Closing

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.

None yet

2 participants