simon-wh commented 5 years ago

Analog SDK Redesign

Introduction

Hi everyone! I'm Simon, working as an Intern for Wooting to redesign the Analog SDK, this is the design I've come up with. Any and all feedback would be greatly appreciated!

Why do we need a redesign?

You can see the discussion of the main issues surrounding the current sdk here at the 'State of the Analog SDK' issue.

Design Overview

Analog SDK Wrapper

This component of the diagram is a wrapper library which wraps around the functions of the Analog SDK at runtime. The reason for this is so that the a developer can link to this wrapper and it can gracefully fail if the Analog SDK is not present on the users system. Additionally, it assists with being able to update the SDK separately, game developers do not need to update anything on their end if the Analog SDK is updated. In theory, the API should be stable, so the only reason a dev would need to update the wrapper is if there are new functions that have been exposed that they want access to. The API exposed from the wrapper will be a C ABI, so most languages should be compatible with the SDK providing they have ffi mechanisms.

Analog SDK

This is where the magic happens. This will be a shared library, that can be independently updated(as mentioned above) which handles loading of analog device plugins, collecting the data from that and providing nice API functions to get analog data in a nice way. I've chosen to write the core SDK in Rust, however, that does not restrict plugins to only Rust. Plugins will be able to be written in any language which can expose a C ABI compatible with the one described later.

Main problems & my proposed solution

Differing keyboard layouts

One of the primary issues we have when it comes to using the SDK is related to keyboard layouts. With the current SDK, let's say you want the analog value for the key A. The way it would respond would be getting the value for the key labelled A in the standard layout (so the key right of caps lock). Now, if you're not English or have some familiarity with differing keyboard layouts you'll see why this is a problem. Not all layouts have 'A' in the same place, for example, the French layout swaps A & Q from the standard layout, so requesting 'A' gets you the value for the input Q. The issue stems from the fundamental question of what information you're looking for. Are you looking for the value of the physical key that is labelled 'A' on the standard layout, or are you looking for the value of the key that inputs 'A' when pressed?

Now this is something where one version is not universal, there are going to be cases where you want from the physical key x, or the key that inputs x. Primarily due to how different game engines handle keyboard input differently. (For those curious, I found a solution for this using the MapVirtualKeyEx WinAPI function which takes Windows virtual keys, so if the keyboard information you have is input key based, then use the VirtualKeys_Locale mode)

Custom keys (those that lie outside of the standard USB HID codes)

One of the problems some of you noticed with the current SDK is the use of a set of Scan codes, identifying each key, which meant nothing outside of the context of the SDK. The reason this was done was due to the existence of keys on the Wooting keyboards that are not part of any standard set of keycodes. Clearly this isn't ideal, as it means that you're pretty quickly going to run into a case where you have to convert they keycode representation you're using to the Wooting set.

There's no perfect solution for this, what I've come up with is to use 2 bytes rather than 1 for the keycodes. With 0x0000 - 0x0100 being the standard keycodes, so nothing is changed there. Custom keys would be anything above or including 0x0100. Alongside this, having a range of the values reserved for common 'custom keys', to keep some consistency between keyboards for keys that appear a lot.

This would apply no matter which set of keycodes you're using to interface with the SDK. Doesn't matter if you're using Windows VirtualKeys or HID codes, up to 0x100 would be that code and above would be the custom key.

EDIT: As @evilC has brought up, the Scan code set 1 uses the 0xE0 prefix (with it being 0x1 in some functions) to represent extended codes to differentiate between left/right keys (e.g. left/right alt). So they will be protected prefixes dedicated for that differentiation with Scan code set 1. So custom keys can use >=0x200 & ~0xE0. There's an edge case with Pause break which I will investigate.

Analog value range

On the existing SDK the analog value that would be reported would be in the range of 0-255. This isn't ideal for this as what if you have a new keyboard which reports more precise analog values? Limiting to the 0-255 range is fairly limiting. We can't just up it to 0-65536 as some could report between 0-255 and some 0-500 etc. To combat this, using a float in the range of 0->1 should suffice, where the plugins handle the appropriate conversions, so all analog values are at the same scale regardless of the precision of the analog reading of the device.

Proposed API

This is the current API design I've come up with. Of course this is subject to change, from your feedback and from changes in design choice that are discovered while implementing.

//NOTE: Function prefixes are ommitted for readability

bool initialise();
bool is_initialised();
bool shutdown();

typedef void (*string_cb)(device_info* info);

typedef struct device_info {
    uint16_t vendor_id;
    uint16_t product_id;
    const char* manufacturer_name;
    const char* device_name;
    tbc device_id;
    //More can be easily added down the line
} device_info;

void set_disconnected_cb(string_cb cb);
void clear_disconnected_cb();

//NOTE: If we use a struct for this, we could just use a const struct and keep updating that rather than allocating a new one everytime which would require freeing after use
//returns the number of devices placed in the buffer, len is the length of the provided buffer
int device_info(device_info* buffer[], int len);

//Version of the API that the SDK is on
int api_version();
//Version of the API that the wrapper is wrapping. i.e. you could use api_version & wrapper_api_version if the wrapper is outdated
int wrapper_api_version();
//The ABI version the SDK is on.
int abi_version();

//HID is the preferred way to interact with the SDK and will be default. Any keycode type on top of that is just there to ease integration for common situations
enum keycode {
    HID,
    ScanCode, // The Scan Codes used in Windows. 'Scan code set 1'
    VirtualKey, //Windows Virtual Keys set (Physical key)
    VirtualKey_Locale //Windows Virtual keys (Locale/Input key)
}

//Set the keycode to be used for the `read_analog` and `read_full_buffer` methods
bool set_mode(keycode code);

//Interprets the code as the keycode set with `set_mode`
float read_analog(uint16_t code);

//The type of keycode returned in this buffer will be the keycode set using `set_mode`
//There are two separate buffers for the codes and the values as using a struct would cause additional issues with releasing the allocated memory
//I'm not the biggest fan of having two arrays, but I'm not sure of a better solution 
int read_full_buffer(uint16_t buffer[], float analog_buffer[], uint8_t length);

//EDIT: Address multiple keyboards individually. Could have these just be the default where -1 indicates any/all
float read_analog_device(uint16_t code, tbc device_id);
int read_full_buffer_device(uint16_t buffer[], float analog_buffer[], uint8_t length, tbc device_id);

Plugin ABI

The plugin ABI is something that doesn't have one single strict definition, as plugins can be written in both C (or anything that can expose this C ABI) & Rust there are going to be some slight differences. However, I'll go over the main functions plugins would have to implement in C style.

Should plugins be required to use a specific hidapi lib to force consistency with devices across platforms? i.e. where usage page numbers are not reported properly by standard hidapi on linux

//Have an 'ABI version' constant that all plugins define so it can be easily checked against the SDK ABI version. This would only be incremented when there are breaking changes or changes that require compatibility for older versions. This allows the SDK to decide the appropriate way to handle the plugins
#define ABI_VERSION 1

int device_info(device_info*[] buffer, int len);

//Self explanitory
bool initialise();
bool is_initialised();
void shutdown();

//This would receive HID codes in the range of 0x0000-0x01000, with anything above being custom defined keys which are outside of the standard keycode range
float read_analog(uint16_t code);
int read_full_buffer(uint16_t[] code_buffer, float[] analog_buffer, uint8_t len);

Updating

The plan for how updates will be handled with the SDK is to leave plugin updating to the manufacturer. For plugins, an environment variable will be used to locate plugins, therefore, manufacturers will be able to add, let's say, a sub-folder of their software installation to that variable which contains their plugin. Thus allowing them to easily update their plugin and the SDK knows where to find it. The wrapper SDK updating will be handled in the way I mentioned [here](#Analog SDK Wrapper).

For the actual SDK though, there are a few ways it could be handled:

It could be updated using a child process that is spawned when the SDK is initialised to check & install SDK updates
It could use a background service that periodically checks for updates and installs them

Beyond the SDK

Once the SDK is complete, the plan is to work on integrating it more closely with popular languages & game engines, to make it a lot easier for devs to quickly pick it up and integrate it into their software.

Questions

Why the choice of using Rust?

Speed, memory safety, good consistent cross platform support, no messy apis like C++

Will this be open-source?

Yes

What platforms will be supported?

Ideally, Windows, Linux & Mac should all be supported, however there may be some differences between platforms with regards to keycodes & layout translations as the layout translations rely on OS specific function calls so are not transferable

What about having a standardised analog usage page?

You can see talk related to having a standardised usage page here. A standardised usage page is theoretically possible, however, it's not on the table for us at the moment. However, we can work on having a standardised layout of the Analog report for now. That paired with a plugin for this system which can read this standardised report, will greatly ease manufacturer integration to this system if they decide to follow the standard system.

Questions for you!

What do you think of the proposed SDK architecture? (i.e. The wrapper + sdk + plugin combo)
What do you think of the API? What would you change? Do you have any concerns?
Are there any additional functions you think should be added to the API?
What do you think about my proposed solutions for our [Main problems](#Main problems & my proposed solution)?

davidtwco commented 5 years ago

I've not had time to read all of this yet, but it's exciting to see the SDK written in Rust. I'd be happy to transfer the crates.io wooting-{analog, rgb}-sdk names to Wooting (that are currently used by my rust-wooting-sdk wrapper project) when the new SDK is ready!

simon-wh commented 5 years ago

@davidtwco That'd be cool! I was planning on just having a C ABI for the wrapper & the SDK, do you think it'd be worthwhile to have a (very stable) Rust interface so Rust applications don't have to go through the C ABI layer?

haata commented 5 years ago

Some areas to consider

How this architecture handles multiple keyboards plugged in at the same time
Two bytes for non-keyboard USB HID codes may work, but you'll have to maintain a mapping somewhere. For HID-IO (and KLL) I developed a database for handling translations between various locales as well as handling the different HID code sets (https://github.com/hid-io/layouts https://github.com/hid-io/layouts/blob/master/base/base.json). Currently I only have Python library to handle the conversions, but the database itself are just a collection of JSON files so using in Rust should be pretty straightforward (https://pypi.org/project/layouts/). Anyways, I would recommend a 3 byte scheme that mirrors the HID spec (or at least a 2 byte scheme that translates easily). For example:

3 byte scheme

<1 byte:type> <2 bytes:code) ``` enum Type { Keyboard, ConsumerCtrl, LED, // This is HID LED, like capslock, not backlighting SystemCtrl, // etc, maybe mouse, and joystick as well } struct { Type type; uint16_t code; } 0x00 0x29 0x00 <- USB (0x00) Esc (0x00 0x29) ``` **2 byte scheme** <1 byte:type> <1 bytes:code) ``` enum Type { Keyboard, ConsumerCtrl1, // Consumer control codes require 2 bytes of namespace (should be possible with 3 types given the current HID spec) ConsumerCtrl2, ConsumerCtrl3, LED, // This is HID LED, like capslock, not backlighting SystemCtrl, // etc, maybe mouse, and joystick as well } struct { Type type; uint16_t code; } 0x00 0x29 0x00 <- USB (0x00) Esc (0x00 0x29) ``` While a bit broader in scope, HID-IO likely try to integrate with the Wooting keyboard interface (or the plugin layer). Also, I do like the conversion of all analog ranges to a 0->1 float (i.e. a percentage).

Rocky04 commented 5 years ago

You need to add the support to differ and address multiple simultaneous keyboards. It would break most likely future compability if this is not considered in the first place. You can differ by hardware if possible with the USB serial number value, by the manufactor which has to provide such informations and at last by the ID of the position of the enumeration (because it can differ from time to time).

haata commented 5 years ago

You need to add the support to differ and address multiple simultaneous keyboards. It would break most likely future compability if this is not considered in the first place. You can differ by hardware if poosible with the USB serial number value, by the manufactor which has to provide such informations and at last by the ID of the position of the enumeration (because it can differ from time to time).

From prior experience, USB serial is good if you are certain it's unique. Falling back to some sort of USB port position might be better though (this is tricky to do). Another thing to consider is if there is a USB error, you may get a disconnection event (which could all happen in fractions of a second).

Rocky04 commented 5 years ago

Falling back to some sort of USB port position might be better though (this is tricky to do).

The easiest way for it can be the possition of the enumeration, you don't really need to get the port number. I guess normally the device is only addressed by the path, so it would add much complexity to get the port number.

davidtwco commented 5 years ago

do you think it'd be worthwhile to have a (very stable) Rust interface so Rust applications don't have to go through the C ABI layer?

@simon-wh I definitely think so. Having a native interface would increase adoption and make integration into other Rust projects far easier and much more likely to happen. Any Rust projects using the C ABI layer would likely need to use unsafe code and make sure to adhere to the invariants of the API - I think it would be worthwhile to wrap this in a safe API that enforces the invariants through the type system (where possible), this is what rust-wooting-sdk does.

simon-wh commented 5 years ago

Some areas to consider

How this architecture handles multiple keyboards plugged in at the same time

Two bytes for non-keyboard USB HID codes may work, but you'll have to maintain a mapping somewhere. For HID-IO (and KLL) I developed a database for handling translations between various locales as well as handling the different HID code sets (https://github.com/hid-io/layouts https://github.com/hid-io/layouts/blob/master/base/base.json). Currently I only have Python library to handle the conversions, but the database itself are just a collection of JSON files so using in Rust should be pretty straightforward (https://pypi.org/project/layouts/). Anyways, I would recommend a 3 byte scheme that mirrors the HID spec (or at least a 2 byte scheme that translates easily).

For the handling of multiple keyboards, me and @Rocky04 discussed on Discord about having the functions include an optional parameter which can be used to say which keyboard/device you want to request from (with nothing/-1 being any/all). I'm thinking that using an identifier which is specific to the particular device would potentially be beneficial in the case where you want to continue requesting from the exact same device if they end up being moved to a different usb port and end up being in a different order in the enumeration.

For the byte scheme, do you think there would be much of a necessity for including the LED(Not sure where that'd be used) & some of the other sections, how much are they used for individual keys? Personally I'd like to try and keep it to a 2-byte code to maximize the amount of keys that can be included in the usage report. But I can see why a 3-byte code would be much easier to use.

@davidtwco Yeah, that makes sense. I'll have to see how feasible it is to setup the wrapper with re-exporting a Rust interface, I do remember seeing a library which could load Rust libraries at runtime & interface with them in a safe way, so should be possible. Might as well have a good Rust interface when we've got the whole thing written in Rust.

evilC commented 5 years ago

There's no perfect solution for this, what I've come up with is to use 2 bytes rather than 1 for the keycodes. With 0x0000 - 0x0100 being the standard keycodes, so nothing is changed there. Custom keys would be anything above or including 0x0100. Alongside this, having a range of the values reserved for common 'custom keys', to keep some consistency between keyboards for keys that appear a lot.

enum keycode {
    HID,
    ScanCode, // The Scan Codes used in Windows. 'Scan code set 1'
    VirtualKey, //Windows Virtual Keys set (Physical key)
    VirtualKey_Locale //Windows Virtual keys (Locale/Input key)
}

Is 0x100 - 0x200 not typically used for extended key codes in Set 1?
https://www.win.tue.nl/~aeb/linux/kbd/scancodes-10.html#scancodesets
eg Right Alt is e0-38 (ie 0x138?)

evilC commented 5 years ago

FYI, both AHK and my GetKeyNameTextW code both seem to agree that the ScanCode for Pause is 0x45

evilC commented 5 years ago

Oh, and also, not quite sure of the suitability of VKs - NumpadHome is 0x67 and Numpad7 is 0x24, yet they are both the same physical key. Do we really wanna deal with NumLock state in this API?

simon-wh commented 5 years ago

Oh, and also, not quite sure of the suitability of VKs - NumpadHome is 0x67 and Numpad7 is 0x24, yet they are both the same physical key. Do we really wanna deal with NumLock state in this API?

If they point to the same key, should we just map them to the same scancode/hid code? The main reason for the VirtualKey inclusion is to deal with the locale translation issues as mentioned in the post, which is something people will run into fairly often, so I believe it's fairly crucial to include it. If we were to decide to get rid of it, we may as well just use the HID codes and not have any other keycode option

WootingKb / wooting-analog-sdk-old