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.

Sign up for GitHub

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

New PR for #19(Watchdog terminates the driver when debugging longer than 10s) #33

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

New PR for #19(Watchdog terminates the driver when debugging longer than 10s) #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions src/VRDriver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@
#include <simdjson.h>
#include "VRPaths_openvr.hpp"

SlimeVRDriver::VRDriver::VRDriver(): m_pBridge(new Bridge(this))
{
}

SlimeVRDriver::VRDriver::~VRDriver()
{
delete this->m_pBridge;
}

vr::EVRInitError SlimeVRDriver::VRDriver::Init(vr::IVRDriverContext* pDriverContext)
{
Expand All @@ -30,11 +38,15 @@ vr::EVRInitError SlimeVRDriver::VRDriver::Init(vr::IVRDriverContext* pDriverCont

Log("SlimeVR Driver Loaded Successfully");

// Start bridge thread
this->m_pBridge->start();

return vr::VRInitError_None;
}

void SlimeVRDriver::VRDriver::Cleanup()
{
this->m_pBridge->stop();
}

void SlimeVRDriver::VRDriver::RunFrame()
Expand All @@ -58,12 +70,11 @@ void SlimeVRDriver::VRDriver::RunFrame()
// Update devices
for(auto& device : this->devices_)
device->Update();

BridgeStatus status = runBridgeFrame(*this);
if(status == BRIDGE_CONNECTED) {

if(this->m_pBridge->state() == Bridge::EState::BRIDGE_CONNECTED) {
messages::ProtobufMessage* message = google::protobuf::Arena::CreateMessage<messages::ProtobufMessage>(&arena);
// Read all messages from the bridge
while(getNextBridgeMessage(*message, *this)) {
while(this->m_pBridge->getNextBridgeMessage(*message)) {
if(message->has_tracker_added()) {
messages::TrackerAdded ta = message->tracker_added();
switch(getDeviceType(static_cast<TrackerRole>(ta.tracker_role()))) {
Expand Down Expand Up @@ -95,13 +106,13 @@ void SlimeVRDriver::VRDriver::RunFrame()
trackerAdded->set_tracker_role(TrackerRole::HMD);
trackerAdded->set_tracker_serial("HMD");
trackerAdded->set_tracker_name("HMD");
sendBridgeMessage(*message, *this);
this->m_pBridge->sendBridgeMessage(*message);

messages::TrackerStatus* trackerStatus = google::protobuf::Arena::CreateMessage<messages::TrackerStatus>(&arena);
message->set_allocated_tracker_status(trackerStatus);
trackerStatus->set_tracker_id(0);
trackerStatus->set_status(messages::TrackerStatus_Status::TrackerStatus_Status_OK);
sendBridgeMessage(*message, *this);
this->m_pBridge->sendBridgeMessage(*message);

sentHmdAddMessage = true;
Log("Sent HMD hello message");
Expand Down Expand Up @@ -166,7 +177,7 @@ void SlimeVRDriver::VRDriver::RunFrame()
hmdPosition->set_qz((float) q.z);
hmdPosition->set_qw((float) q.w);

sendBridgeMessage(*message, *this);
this->m_pBridge->sendBridgeMessage(*message);
} else {
// If bridge not connected, assume we need to resend hmd tracker add message
sentHmdAddMessage = false;
Expand Down
5 changes: 4 additions & 1 deletion src/VRDriver.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
#include <simdjson.h>

namespace SlimeVRDriver {
class Bridge;
class VRDriver : public IVRDriver {
public:
VRDriver();
~VRDriver() override;

// Inherited via IVRDriver
virtual std::vector<std::shared_ptr<IVRDevice>> GetDevices() override;
Expand All @@ -35,7 +38,6 @@ namespace SlimeVRDriver {
virtual bool ShouldBlockStandbyMode() override;
virtual void EnterStandby() override;
virtual void LeaveStandby() override;
virtual ~VRDriver() = default;

virtual std::optional<UniverseTranslation> GetCurrentUniverse() override;

Expand All @@ -52,6 +54,7 @@ namespace SlimeVRDriver {
vr::HmdVector3_t GetPosition(vr::HmdMatrix34_t &matrix);

bool sentHmdAddMessage = false;
Bridge *m_pBridge;
Copy link
Member

Choose a reason for hiding this comment

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

Currently, we do not use (systems) hungarian notation in the driver. To be fair though, this is somewhat of a nitpick.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Bridge *m_pBridge;
Bridge *bridge;

should've included the suggestion in the original comment.

I don't want to make suggestions for every occurrence, because basically every single variable/parameter that you added does not follow the style of the rest of the driver, and I don't want to spam the PR.


simdjson::ondemand::parser json_parser;
std::optional<std::string> default_chap_path_ = std::nullopt;
Expand Down
162 changes: 111 additions & 51 deletions src/bridge/bridge-unix-sockets.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* on unix sockets
*/
#include "bridge.hpp"
#include "../VRDriver.hpp"
#ifdef __linux__
#include "unix-sockets.hpp"
#include <string_view>
Expand Down Expand Up @@ -73,98 +74,157 @@ BasicLocalClient client{};
inline constexpr int BUFFER_SIZE = 1024;
using ByteBuffer = std::array<uint8_t, BUFFER_SIZE>;
ByteBuffer byteBuffer;
}

SlimeVRDriver::Bridge::Bridge(VRDriver *i_pDriver)
: m_pDriver(i_pDriver), m_pPipe(nullptr), m_pBuffer(nullptr),
m_eState(EState::BRIDGE_DISCONNECTED), m_bStop(false) {
}

SlimeVRDriver::Bridge::~Bridge() {
this->stop();
}

void SlimeVRDriver::Bridge::run() {
Copy link
Member

Choose a reason for hiding this comment

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

This appears to consist entirely of platform independent code, but is duplicated between windows pipes and unix sockets.


messages::ProtobufMessage oMsg;

while (!this->m_bStop) {
std::this_thread::sleep_for(std::chrono::milliseconds(3));
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, we'd wait for the OS to notify us that there's a message incoming, or we have a message in the queue that needs to be sent instead of sleeping for <arbitrary amount of time>.

Copy link
Author

Choose a reason for hiding this comment

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

The sleep is a common practice to allow the cpu to switch the context to avoid high utilization.

In my eyes it would be the best to remove the pipes completely and send the data over TCP e.g. with Asio. Then you have only one implementation that works for windows and linux.

Copy link
Member

Choose a reason for hiding this comment

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

I'm talking about using select or WaitForMultipleObjects instead of sleeping, meaning the OS can wake us up as soon as either there's an incoming message, or another thread sends a notification that there's an outgoing message, and we don't need to periodically wake-up to poll the outgoing queue.

This would still be applicable to a TCP based implementation, unless you had the other thread just send the message directly. (But that optimization would also apply to this implementation, so eh)

This doesn't need to be done right now, so I'll resolve the conversation after adding a TODO.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
std::this_thread::sleep_for(std::chrono::milliseconds(3));
// TODO: use select, or a timeout, or just send the messages from another thread.
std::this_thread::sleep_for(std::chrono::milliseconds(3));


switch (this->m_eState) {
case EState::BRIDGE_DISCONNECTED:
attemptPipeConnect();
break;
case EState::BRIDGE_ERROR:
resetPipe();
break;
case EState::BRIDGE_CONNECTED:
default:
break;
}

client.UpdateOnce();

// Read all msg
while (this->fetchNextBridgeMessage(oMsg)) {
std::lock_guard<std::recursive_mutex> lk(this->m_oMutex);
this->m_aRecvQueue.push(std::move(oMsg));
};


while (true) {
{ // Custom scope for send queue mutex
std::lock_guard<std::recursive_mutex> lk(this->m_oMutex);
if (this->m_aSendQueue.empty()) {
break;
}

oMsg = std::move(this->m_aSendQueue.front());
this->m_aSendQueue.pop();
}

this->sendBridgeMessageFromQueue(oMsg);
}

}
}
void SlimeVRDriver::Bridge::sendBridgeMessageFromQueue(messages::ProtobufMessage &i_oMessage) {
if (!client.IsOpen()) {
this->m_pDriver->Log("bridge send error: failed to write header");
this->setBridgeError();
};

const auto bufBegin = byteBuffer.begin();
const auto bufferSize = static_cast<int>(std::distance(bufBegin, byteBuffer.end()));
const auto msgSize = static_cast<int>(i_oMessage.ByteSizeLong());
const std::optional msgBeginIt = WriteHeader(bufBegin, bufferSize, msgSize);
if (!msgBeginIt) {
this->m_pDriver->Log("bridge send error: failed to write header");
return;
}
if (!i_oMessage.SerializeToArray(&(**msgBeginIt), msgSize)) {
this->m_pDriver->Log("bridge send error: failed to serialize");
return;
}
int bytesToSend = static_cast<int>(std::distance(bufBegin, *msgBeginIt + msgSize));
if (bytesToSend <= 0) {
this->m_pDriver->Log("bridge send error: empty message");
return;
}
if (bytesToSend > bufferSize) {
this->m_pDriver->Log("bridge send error: message too big");
return;
}
try {
client.Send(bufBegin, bytesToSend);
} catch (const std::exception& e) {
this->setBridgeError();
this->m_pDriver->Log("bridge send error: " + std::string(e.what()));
return;
}
}

bool getNextBridgeMessage(messages::ProtobufMessage& message, SlimeVRDriver::VRDriver& driver) {
bool SlimeVRDriver::Bridge::fetchNextBridgeMessage(messages::ProtobufMessage &i_oMessage) {
if (!client.IsOpen()) return false;

int bytesRecv = 0;
try {
bytesRecv = client.RecvOnce(byteBuffer.begin(), HEADER_SIZE);
} catch (const std::exception& e) {
client.Close();
driver.Log("bridge send error: " + std::string(e.what()));
this->m_pDriver->Log("bridge send error: " + std::string(e.what()));
return false;
}
if (bytesRecv == 0) return false; // no message waiting

int msgSize = 0;
const std::optional msgBeginIt = ReadHeader(byteBuffer.begin(), bytesRecv, msgSize);
if (!msgBeginIt) {
driver.Log("bridge recv error: invalid message header or size");
this->m_pDriver->Log("bridge recv error: invalid message header or size");
return false;
}
if (msgSize <= 0) {
driver.Log("bridge recv error: empty message");
this->m_pDriver->Log("bridge recv error: empty message");
return false;
}
try {
if (!client.RecvAll(*msgBeginIt, msgSize)) {
driver.Log("bridge recv error: client closed");
this->m_pDriver->Log("bridge recv error: client closed");
return false;
}
} catch (const std::exception& e) {
client.Close();
driver.Log("bridge send error: " + std::string(e.what()));
this->m_pDriver->Log("bridge send error: " + std::string(e.what()));
return false;
}
if (!message.ParseFromArray(&(**msgBeginIt), msgSize)) {
driver.Log("bridge recv error: failed to parse");
if (!i_oMessage.ParseFromArray(&(**msgBeginIt), msgSize)) {
this->m_pDriver->Log("bridge recv error: failed to parse");
return false;
}

return true;
}

bool sendBridgeMessage(messages::ProtobufMessage& message, SlimeVRDriver::VRDriver& driver) {
if (!client.IsOpen()) return false;
const auto bufBegin = byteBuffer.begin();
const auto bufferSize = static_cast<int>(std::distance(bufBegin, byteBuffer.end()));
const auto msgSize = static_cast<int>(message.ByteSizeLong());
const std::optional msgBeginIt = WriteHeader(bufBegin, bufferSize, msgSize);
if (!msgBeginIt) {
driver.Log("bridge send error: failed to write header");
return false;
}
if (!message.SerializeToArray(&(**msgBeginIt), msgSize)) {
driver.Log("bridge send error: failed to serialize");
return false;
}
int bytesToSend = static_cast<int>(std::distance(bufBegin, *msgBeginIt + msgSize));
if (bytesToSend <= 0) {
driver.Log("bridge send error: empty message");
return false;
}
if (bytesToSend > bufferSize) {
driver.Log("bridge send error: message too big");
return false;
}
try {
return client.Send(bufBegin, bytesToSend);
} catch (const std::exception& e) {
client.Close();
driver.Log("bridge send error: " + std::string(e.what()));
return false;
}
void SlimeVRDriver::Bridge::setBridgeError() {
this->m_eState = EState::BRIDGE_ERROR;
}

BridgeStatus runBridgeFrame(SlimeVRDriver::VRDriver& driver) {
void SlimeVRDriver::Bridge::resetPipe() {
try {
if (!client.IsOpen()) {
client.Open(SOCKET_PATH);
}
client.UpdateOnce();

if (!client.IsOpen()) {
return BRIDGE_DISCONNECTED;
}
return BRIDGE_CONNECTED;
client.Close();
client.Open(SOCKET_PATH);
this->m_pDriver->Log("Pipe was reset");
} catch (const std::exception& e) {
client.Close();
driver.Log("bridge error: " + std::string(e.what()));
return BRIDGE_ERROR;
this->m_pDriver->Log("bridge error: " + std::string(e.what()));
setBridgeError();
}

}
void SlimeVRDriver::Bridge::attemptPipeConnect() {
// Open pipe only called the first time
this->resetPipe();
}


#endif // linux
Loading