Implement async IPC#38
Conversation
# Conflicts: # src/bridge/bridge-unix-sockets.cpp
# Conflicts: # .github/workflows/c-cpp.yml
# Conflicts: # .github/workflows/c-cpp.yml
There was a problem hiding this comment.
Sorry it's taken so long for me to review this. Overall, I think you've raised the code quality of the driver, even if I wish some of the style/formatting changes had been done in a separate PR to make this one easier to review. (However, I never spoke up previously to comment on or ask for that, so that's on me.) Almost every time I went "wait, that's weird" it ended up being original code that was reformatted, or a quirk of the original code that was faithfully reproduced, and I'm not going to argue with that.
None of my review comments block this PR. (but to be clear, I would appreciate some response.) They fall in a few categories:
- Questions that I'd like an answer for, but require no action and I won't loose sleep over not knowing.
- nits about byteswap that I'd prefer were addressed, but are ultimately unimportant. (it'll work either way.)
- pointing out things that others might find interesting.
There are a couple limitations to what I've reviewed at this time:
- I haven't viewed libuv documentation, so I'm basically going off of vibes, and taking it on faith since there are multiple reports of this code working.
- Similarly, I noticed that you've started using atomics in some places to account for the newly multi-threaded nature of this code. I don't have the spoons to check if you've missed anything. (I haven't done any multithreaded programming in C++, and Rust will largely just fail to compile if you haven't justified yourself correctly.)
- I've mostly glossed over the test code.
|
Tracker reconnection/readding broke after reverting Originally on each reconnection there was a new device instance created with these flags set to |
IO is now handled by libuv. Pose requests now happen at 400-500hz to minimize latency, decoupled from
RunFrame.Tested with ALVR, WMR on Windows combined with SlimeVR/SlimeVR-Server#629.Reported by other testers that works on Linux.Needs more testing on Linux.Beta checklist
OS:
HMD driver: