Add XPG Prime support

[XPG-NPC]

This pull request proposes the following changes:

• Add support for the Summoner and Mage keyboards.
• Add a common layout for those keyboards.

[diogotr7]

Is there any specific reason this is implemented as a ScriptedDevice? Have you looked into the DefaultDevice class? I wrote it as an easier to implement alternative to IDevice.

Does this rely on the order of the deviceKeys dictionary to send colors to the device? This will result in the wrong colors being sent if the user has the wrong layout selected (which is not necessatily going to happen but should be protected against anyway).

[XPG-NPC]

Is there any specific reason this is implemented as a ScriptedDevice? Have you looked into the DefaultDevice class? I wrote it as an easier to implement alternative to IDevice.

Not a technical reason.
It is my fault for developping this against the master branch where DeviceScript was the easier to implement alternative.
I didn't notice DefaultDevice. I see now how it would be a bit more convenient.
I'll get on it.

Does this rely on the order of the deviceKeys dictionary to send colors to the device? This will result in the wrong colors being sent if the user has the wrong layout selected (which is not necessatily going to happen but should be protected against anyway).

The order in the dictionary doesn't matter because each keyboard key uses the color of a single corresponding DeviceKey.

[diogotr7]

Ah, you're right, I missed int k = (int)kvp.Key;.

I might try and port this to Artemis as well. Was this dll written specifically for Aurora? I assume there are no docs available apart from this implementation? Doesn't look too complicated, thankfully.

[diogotr7]

I wrote an initial version of an Artemis plugin for these devices: https://github.com/diogotr7/Artemis.Plugins.Devices.XpgPrime

If there's any chance you could test this i would appreciate it. After all the more choice the user gets, the better.

[diogotr7]

Any update on this?

[XPG-NPC]

I might try and port this to Artemis as well. Was this dll written specifically for Aurora? I assume there are no docs available apart from this implementation? Doesn't look too complicated, thankfully.

That's great!
And you are right on both accounts.

Any update on this?

The XPGDevice using DefaultDevice and the one using DeviceScript are very similar. However I encountered an issue when doing the switch. In some cases, Aurora stops abruptly shortly after updating the device. My first idea was that I did something wrong with the calling convention or marshalling and, indeed, this is an error 0xc0000005 (Access Violation): "Common sources of this bug include user marshaling errors for COM-interop or PInvoke, which may corrupt the stack."

C# is not my strong suit. The crash happens or not depending on how I call UpdateDevice(). For instance, it's called from a try-catch, the crash disappears without any exception getting caught. The DLL interface was written in C, with UpdadeDevice() taking a void* pointer. Using unsafe and fixed to explicitly use a pointer works (XPGDevice_unsafe-fixed.txt). However, my understanding is that using AddrOfPinnedObject() or Marshal.UnsafeAddrOfPinnedArrayElement should work too, yet the crash comes back if I also use GCHandle to pin the memory address as required.

Any idea what exactly is going wrong? I have code that seems to be working but I'd like to understand to avoid any latent issue.

I wrote an initial version of an Artemis plugin for these devices: https://github.com/diogotr7/Artemis.Plugins.Devices.XpgPrime

If there's any chance you could test this i would appreciate it. After all the more choice the user gets, the better.

I did give it a brief look. I didn't want to spend time on this before I understand the issue above. I'll give feedback somewhere than this pull request.

[diogotr7]

These access violation exceptions cannot be caught. It's not clear to me what changed exactly between the old implementation and the new one that made it break. I don't think this would be a threading issue since DeviceManager locks every operation on IDevice. Did the Scripted Device implementation ever have this issue? I assume you didn't change the native dll in the meantime. If you did, try testing with the old one.

I'm not a fan of the use of unsafe, it shouldn't be needed. Several other DllImports in Aurora have byte arrays as parameters without issue.

[XPG-NPC]

These access violation exceptions cannot be caught. It's not clear to me what changed exactly between the old implementation and the new one that made it break. I don't think this would be a threading issue since DeviceManager locks every operation on IDevice. Did the Scripted Device implementation ever have this issue? I assume you didn't change the native dll in the meantime. If you did, try testing with the old one.

I'm not a fan of the use of unsafe, it shouldn't be needed. Several other DllImports in Aurora have byte arrays as parameters without issue.

I think I finally pinned down the issue.
Apparently mingw's alloca does something to the stack that the C# runtine doesn't like.
I'll update the branch on monday.

[XPG-NPC]

XPGDevice now inherits from DefaultDevice.

The DLL has been updated to avoid the crash.
It also uses a more recent hidapi version that fixes a few rare bugs on Windows.