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

Implemented GC Controller inputs via named pipes. #3170

Merged
merged 1 commit into from Oct 28, 2015

Conversation

7 participants
@spxtr
Copy link
Member

spxtr commented Oct 14, 2015

When we look for controllers, Pipes::Init scans the Pipes folder in the user directory for readable, nonblocking files. For those it finds, it creates ciface::Devices out of them. Users can then send controller inputs to the devices by writing to the files. This is much simpler than, e.g. making a uinput device. The commands are described in Pipes.h.

This approach can be extended to wiimotes. You can also pipe data from a socket into the file with netcat or similar.

Only works on unix systems so far, since Windows has a different named pipes interface. Not a big deal to port, but I don't have a Windows box atm. Try it out by making a named pipe in a folder called Pipes in the user directory (mkfifo filename), refresh dolphin controller entries, pick the pipe, and then run

echo "PRESS A" > filename
echo "RELEASE A" > filename
{
namespace Pipes
{
static std::vector<std::string> s_button_tokens {

This comment has been minimized.

@degasus

degasus Oct 14, 2015

Member

const, also for the next three ones

@MayImilae

This comment has been minimized.

Copy link
Contributor

MayImilae commented Oct 14, 2015

Would this have any effect on regular users? Any GUI or configuration process differences?

while (newline != std::string::npos) {
std::string command = m_buf.substr(0, newline);
ParseCommand(command);
m_buf.erase(0, newline + 1);

This comment has been minimized.

@degasus

degasus Oct 14, 2015

Member

I doubt performance matters, but this line end in N² complexity for lots of input in the queue. It would likely be better to parse the input on every read but to try to read everything first.

@@ -25,6 +25,10 @@ if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux" AND NOT ANDROID)
option(ENABLE_EVDEV "Enables the evdev controller backend" ON)
endif()

if(UNIX)
option(ENABLE_PIPES "Enable named pipe controller input" ON)
endif()

This comment has been minimized.

@Tilka

Tilka Oct 14, 2015

Member

I don't think this needs an option.

"C"
};

void Init(std::vector<Core::Device*> &devices)

This comment has been minimized.

@Tilka

Tilka Oct 14, 2015

Member

Ampersand against type please. Same below.

while (bytes_read > 0) {
m_buf.append(buf, bytes_read);
bytes_read = read(m_fd, buf, sizeof buf);
}

This comment has been minimized.

@degasus

degasus Oct 14, 2015

Member

I guess this code has less redundancy:

while (true)
{
  char buf[32];
  ssize_t bytes_read = read(m_fd, buf, sizeof buf);
  if (bytes_read <= 0)
    break;
  m_buf.append(buf, bytes_read);
}

#include "InputCommon/ControllerInterface/Pipes/Pipes.h"
#include "Common/FileUtil.h"
#include "Common/StringUtil.h"

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

These should all be sorted alphabetically.

@lioncash

This comment has been minimized.

Copy link
Member

lioncash commented Oct 14, 2015

Braces should be on the next line

if (value < 0.0)
value = 0.0;
if (value > 1.0)
value = 1.0;

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

MathUtils::Clamp


#include <string>
#include <vector>
#include <map>

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

These should be sorted alphabetically


class PipeDevice : public Core::Device
{
private:

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

Join this with the other private specifier (it would go above the function declarations)

void SetState(ControlState state) { _state = state; }
private:
const std::string _name;
ControlState _state;

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

m_state and m_name

m_buf.append(buf, bytes_read);
bytes_read = read(m_fd, buf, sizeof buf);
}
std::size_t newline = m_buf.find_first_of("\n");

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

std::size_t newline = m_buf.find('\n'); Same for the occurrence below this

}

PipeDevice::PipeDevice(int fd, const std::string &name, int id)
: m_fd(fd), m_name(name), m_id(id), m_buf("")

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

m_buf doesn't need to be here. strings initialize to empty string by default.

@spxtr

This comment has been minimized.

Copy link
Member

spxtr commented Oct 14, 2015

Okay, I made those changes. I'm not sure if I put the big comment in the right spot in Pipes.h.

#include <iostream>
#include <map>
#include <string>
#include <sys/stat.h>

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

This goes after vector lexicographically

std::map<std::string, PipeInput*> m_buttons;
std::map<std::string, PipeInput*> m_axes;

public:

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

Public is supposed to be before private members

class PipeInput : public Input
{
public:
std::string GetName() const { return m_name; }

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

GetName should have an override specifier after the const.

public:
std::string GetName() const { return m_name; }
PipeInput(std::string name) : m_name(name), m_state(0.0) {}
ControlState GetState() const { return m_state; }

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

GetState should have an override specifier after the const

{
public:
std::string GetName() const { return m_name; }
PipeInput(std::string name) : m_name(name), m_state(0.0) {}

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

Constructor should be the first thing listed among the public members. std::string should be passed by const reference.

void UpdateInput() override;

PipeDevice(int fd, const std::string& name, int id);
~PipeDevice();

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

Constructor and destructor should be the first thing in the public section of the class

return;
for (unsigned int i = 0; i < fst.size; ++i)
{
File::FSTEntry child = fst.children[i];

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

This can be a const reference.

ControlState m_state;
};

void AddAxis(std::string name, double value);

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

Pass string by const reference

};

void AddAxis(std::string name, double value);
void ParseCommand(std::string command);

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

Pass string by const reference


void AddAxis(std::string name, double value);
void ParseCommand(std::string command);
void SetAxis(std::string entry, double value);

This comment has been minimized.

@lioncash

lioncash Oct 14, 2015

Member

Pass string by const reference

@spxtr

This comment has been minimized.

Copy link
Member

spxtr commented Oct 14, 2015

Sorry about the plethora of style issues, by the way. I copied bits and pieces from the evdev and Android interfaces, which have some style issues that I should've noticed and avoided.

"R"
};

static const std::vector<std::string> s_axis_tokens

This comment has been minimized.

@lioncash

lioncash Oct 25, 2015

Member

these should be std::array

GC controller input using named pipes
Currently only works on unix, but can be extended to other systems. Can
also be extended to do wiimotes.

Searches the Pipes folder for readable named pipes and creates a dolphin
input device out of them. Send controller inputs to the game by writing
to the file. Commands are described in Pipes.h.
@phire

This comment has been minimized.

Copy link
Member

phire commented Oct 25, 2015

@dolphin-emu-bot rebuild

This looks good to me.

@dolphin-emu-bot

This comment has been minimized.

Copy link
Contributor

dolphin-emu-bot commented Oct 25, 2015

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • fortune-street-white-box on ogl-lin-intel: diff
  • mkdd-efb on ogl-lin-intel: diff
  • sms-bubbles on ogl-lin-intel: diff
  • ssbm-pointsize on ogl-lin-intel: diff

automated-fifoci-reporter

lioncash added a commit that referenced this pull request Oct 28, 2015

Merge pull request #3170 from spxtr/pipes
Implemented GC Controller inputs via named pipes.

@lioncash lioncash merged commit e76b1f2 into dolphin-emu:master Oct 28, 2015

11 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on the Buildbot.
Details
pr-android Build succeeded on the Buildbot.
Details
pr-deb-dbg-x64 Build succeeded on the Buildbot.
Details
pr-deb-x64 Build succeeded on the Buildbot.
Details
pr-freebsd Build succeeded on the Buildbot.
Details
pr-osx-x64 Build succeeded on the Buildbot.
Details
pr-ubu-nogui-x64 Build succeeded on the Buildbot.
Details
pr-ubu-x64 Build succeeded on the Buildbot.
Details
pr-win-dbg-x64 Build succeeded on the Buildbot.
Details
pr-win-x64 Build succeeded on the Buildbot.
Details

@spxtr spxtr referenced this pull request Mar 27, 2017

Closed

Use dolphin's MemoryWatcher #23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment