Agneli / linux-remote-control

Turn any device into a complete remote control for your GNU/Linux
www.linuxremotecontrol.com
GNU General Public License v3.0
71 stars 21 forks source link

[Question] Use websockets for all communications? #20

Open lufte opened 10 years ago

lufte commented 10 years ago

I would like to know Raphael's and everyone else opinion on this before doing anything: should we use only a websocket for all client-server communications? In my opinion, a websocket is more suited for an app like this than http requests.

Of course, it's a major change and most of the server code would have to be re-written.

Agneli commented 10 years ago

Hello Javier. Do not know much about Websockets, but what little I understand it enough to know that it is better than the current ajax request that is in the application.

As you are expert. Feel free to do the way you think best.

As for the idea of preventing the user from moving if not connected, is good also. Since a message telling the user what problem is occurring is displayed.

Anyway, Feel free to make the necessary changes using Websockets.

Thank you.

316k commented 10 years ago

That is definitely a good idea. It would be the perfect opportunity to redesign the structure of the application, which could be a little more organized. I trust you to keep the new structure simple and generic.

(My comment is a week late, have you started yet ?)

lufte commented 10 years ago

I haven't started yet. I started some improvements for the touchpad (acceleration, two finger scrolling and click by tapping) that I have yet to finish before working on this. I will come up with a design before changing anything so we can all agree on it.

316k commented 10 years ago

Sweet, can't wait to see those improvements ! In the meantime, I'll enthusiastically wait for your design :)

316k commented 10 years ago

Do you think it would be possible/a good choice to have multiple drivers for the communication with the server ? I'm actually thinking about Bluetooth communication, which will probably be supported by standard HTML5 in a future version : https://developer.mozilla.org/en-US/docs/Web/API/Web_Bluetooth_API

Also, this project aims to support Bluetooth for NodeJS : https://github.com/eelcocramer/node-bluetooth-serial-port

There is currently a non-standard way to implement Bluetooth communication in a Firefox OS app, but it's only available for certified apps (usually, the native and non-uninstallable ones). There is a hacky way to install your own certified apps on a phone, but you have to edit a json config file in your phone through the adb shell (that won't go in the marketplace).

Still, it would be nice to eventually have an app that is not network-dependant.

lufte commented 10 years ago

I would love this. When I take the laptop to my bedroom to watch a movie in bed (best use case for a remote control app :) I don't have enough wifi signal, so bluetooth would be perfect. I didn't know it wasn't possible yet, too bad.

316k commented 10 years ago

Well, we could temporarily implement the hacky-version with a userguide to explain how to use it, until the standard way to do it is ready... The only implication would be to have a more abstract structure for the communication protocol.

Agneli commented 10 years ago

@lufte, how is the implementation of the Websockets ? Is giving all right ?

lufte commented 10 years ago

I've been busy lately so I couldn't make progress on this, but here's the design I have in mind:

Use a string of the form [preffix][suffix] to send something from the client to the server.

[prefix] is a string with fixed length (even 1 character is enough) that identifies the kind of request the client is making. We write a set of constants (I already wrote a few for the touchpad) to identify several requests like move cursor, music-related request, shutdown computer, etc... These constants will have to be defined both in the client code and the server code, and it's important that both stay consistent. When the server receives the request it evaluates the prefix to handle it with the appropiate code (music_manager, touch.js and so).

[suffix] can be anything. For complex requests like track information in the music manager it can be a json string, and for simple and speed-dependant functions like the touchpad it is a short string with two numbers separated by a semi colon. The piece of code that handles the request will know how to process the suffix.

Sometimes, the server will have to send something as a response to the client. The client will know when this has to happen and will handle it accordly. Some other times, the server will have to send something to the client that is not exactly a response, like when a track finishes in the music player. I'm not sure if this will need a different socket that listens on the client or not.

Thoughts?

316k commented 10 years ago

If I understand this correctly, websockets allow to send simple messages as strings. If this is the only way to transmit information through websockets, then the prefix/suffix solution is the only one.

What about that for a convention : "function_name/all the information the function need..."

Currently, a switch is used to determine what to do depending on the prefix sent. But we're writing Javascript code, so keep in mind the answer to all of our problems is JSON. Instead of defining constants binded to a switch for every action, what would you think of a structure like :

var actions = {
    // Mouse clicks (only one letter to ensure the message sent is small)
    c: function(parameters) {
        // Things to do on Click event
    },
    // Mouse movements
    m: function(parameters) {
        // Things to do on Move event
    },
    music: function(parameters) {
        ...
    }
}

wss.on('connection', function(ws) {
    ws.on('message', function(message) {
        var action = message.split('/')[0];
        var parameters = message... // Everything but the prefix
        actions[action](parameters); // sends
    });
});

There would be something similar on the client side, since Websockets get rid of the Client vs Server concept. Then, when the server starts, it should start to periodically check some status to send to the client, such as music informations, backlight status, ...

function infos() {
    exec("amixer sget Master | grep '%]'", function(...) {
        // If it's different than it was last time, send a ws:// message to the clients
    });
    exec(...)
}
setInterval(infos, 1000);

Question : @lufte how do we deal with multiple clients and websockets ? Does the code for the touchpad work if multiple clients try to send requests to the server at the same time ?

Also, is there a way to return a value after a precise message has been sent (not asynchronously) with websockets ?

lufte commented 10 years ago

This approach is definitely more "javascript-ish" than mine :). I like it. I have my doubts about using split because it's less performant than extracting a fixed length string, but everything else looks great.

About the websocket questions:

  1. I'm not really sure how different websockets are from normal sockets. Usually, when you connect a listening socket it stops listening to further connections (I assume this is what we want), but maybe it's different with web sockets.
  2. I don't think so, we either implement our own blocking system or just wait for an asynchronous response.
316k commented 10 years ago

Yeah... I don't like split either (then, we'd have to re-join() the rest of the message and re-split it in the function...), but having a fixed length prefix seems less flexible. How less performant is it ? Have you done some tests to see if it's significant in the context of touch functions ? (This is definitively the thing that will need the speediest handling)

lufte commented 10 years ago

I ran some tests and split is quite slower when the string becomes larger. With over 100 characters it takes up to decimals of a second to complete.

Then it occurred to me that we can use string.indexOf and substring. It doesn't depend on the length of the whole string (just the prefix, but I guess we won't be using function names with over 100 characters :) and we still can use variable length prefixes. What do you say?

316k commented 10 years ago

Yup, sounds good. Let's do this.

316k commented 10 years ago

@lufte How is it going ? Any development ?

lufte commented 10 years ago

Uf, sorry guys, I started working on a different project and haven't had time to work on this. Please, if someone else wants to implement it, go ahead. I don't want to hold this back.

Sorry I didn't let you know before.

316k commented 10 years ago

It's ok, I thought so ;) I think I'll give it a shot, WebSockets sound quite interesting to me. I just wanted to be sure you didn't begin anything before I started implementing it, to avoid clashes with your potential work.

lufte commented 10 years ago

Of course. If you have used sockets in other languages you'll have no problem. If you haven't, websockets are actually simpler.

I used this link to get started: https://developer.mozilla.org/en/docs/WebSockets.

Agneli commented 10 years ago

Without problems @lufte. You contributed much with LRC. I thank you for that. Hope you come back to contribute when you can. Good lucky on your new project. Thank you very much.

316k commented 10 years ago

Ok, I made some changes in my feature/websocket-everywhere branch.

I created a class Connection with subclasses for connection drivers (Connection_HTTP and Connection_WebSocket). Connections drivers must implement the send(fct, arguments) function and they have their own way to refresh the view (callback for WebSocket, setInterval with jsonp for HTTP).

Instead of doing :

    $.get("http://" + navigator.host + ":" + port + "/music", {action: $(this).data("action")});

We have to call :

    connection.send('music', {action: $(this).data("action")});

Which will send a request using $.get or websocket.send, depending on the selected driver.

This structure might allow us to simply implement a Bluetooth connection in the future.

AlexMl commented 10 years ago

sounds very good to me

316k commented 10 years ago

More modifications in my feature/websocket-everywhere branch. I changed the structure of the code of the server, and now both HTTP and WebSocket connections are possible.

To implement a new action, add a function in the big var actions at the beginning of the lrc.js script :

var actions = {
    /**
      * Called by HTTP driver for a URL such as : http://myserver/info?my_parameter=stuff&another_parameter=123
      * Called by WebSocket driver when a message such as : 'info/{"my_parameter": "stuff", "another_parameter": 123}' is received
      * parameters
      */
    info: function(parameters, callback) {
        exec("amixer sget Master | grep '%]' && xbacklight -get", function(error, stdout, stderr) {
            // Do stuff
            var data = {
                volume: 35,
                backlight: parameters.another_parameter,
            };

            /**
             * Callback is defined by connection drivers. It sends a value to the client
             * for HTTP, it's : res.send(data),
             * for WebSocket, it's wss.broadcast(data)
             */
            callback(data);
        });
    },
    // ...

The client and the server aren't yet perfectly coherent, but this giant issue that's blocking almost every other feature from being implemented will soon be closed :)

316k commented 10 years ago

@Agneli : Can you investigate what makes the client app so slow ? I can't reproduce it. Try it in both your phone and your desktop browser if you can.