Api V2 suggestion (:

[ghost]

I briefly mentioned this to @cprezzi and he mentioned @tbfleming might anyway be revamping this for his new UI work, but i figured let me write up what i kinda need (using lw.comms-server in another frontend more config orientated) and see what you guys think before i forge ahead with my needs and this never makes it into lw.comm-server (;

Right now - my old legacy way of doing that has stuck inside lw for too long, has been to send individual websocket events for position, queuecnt, etc - very inefficient. in the frontend we have to listen for various named ws events...

I propose we convert to a single object that gets sent over as a single websocket event. Data binding to a single object and then just updating UI when a key changes is more efficient and easier to debug:

Here's what i have so far

var statusjson = {
  machine: {
    overrides: {
      feedOverride: 0,
      spindleOverride: 0
    },
    position: {
      work: {
        xWpos: 0,
        yWpos: 0,
        zWpos: 0,
        aWpos: 0,
        eWpos: 0,
      },
      machine: {
        xMpos: 0,
        yMpos: 0,
        zMpos: 0,
        aMpos: 0,
        eMpos: 0
      }

    },
    temperature: {
      t1temp: 0,
      t2temp: 0,
      bedtemp: 0,
    },
    firmware: {
      type: "",
      version: "",
      date: ""
    },
    configuration: {
      x_steps_per_mm: 0,
      y_steps_per_mm: 0,
      z_steps_per_mm: 0,
      x_acceleration: 0,
      y_acceleration: 0,
      z_acceleration: 0,
      x_max_rate: 0,
      y_max_rate: 0,
      z_max_rate: 0,
      x_axis_length: 0, // Max lenght of axes: Used for soft limits in grbl and smoothie - also maybe useful to autodraw grid size
      y_axis_length: 0, // Max lenght of axes: Used for soft limits in grbl and smoothie - also maybe useful to autodraw grid size
      z_axis_length: 0, // Max lenght of axes: Used for soft limits in grbl and smoothie - also maybe useful to autodraw grid size
      x_homing: 0, // 0 = none, 1 = min, 2 = max
      y_homing: 0, // 0 = none, 1 = min, 2 = max
      z_homing: 0, // 0 = none, 1 = min, 2 = max
      x_home_invert: 0, // 0 = none, 1 = inverted
      y_home_invert: 0, // 0 = none, 1 = inverted
      z_home_invert: 0, // 0 = none, 1 = min, 2 = max
      x_dir: 0, // 0 = normal, 1 = inverted
      y_dir: 0, // 0 = normal, 1 = inverted
      z_dir: 0, // 0 = normal, 1 = inverted
      x_current: 0, // grbl-lpc and smoothie
      y_current: 0, // grbl-lpc and smoothie
      z_current: 0, // grbl-lpc and smoothie
      tools: {
        bed: true, // not really scoped to configure from here... but rather used in frontend to configure showing options related
        e0: true, // not really scoped to configure from here... but rather used in frontend to configure showing options related
        e1: false, // not really scoped to configure from here... but rather used in frontend to configure showing options related
        laser: false, // not really scoped to configure from here... but rather used in frontend to configure showing options related
        spindle: false // not really scoped to configure from here... but rather used in frontend to configure showing options related
      },
    }
  },
  comms: {
    connectionStatus: 0, //0 = not connected, 1 = connected, 2 = playing, 3 = paused, etc?
    runStatus: 0,// 0 = init, 1 = idle, 2 = alarm, 3 = stop, 4 = run, etc?
    queue: 0,
    controllerBuffer: 0, // Seems like you are tracking available buffer?  Maybe nice to have in frontend?
    interfaces: {
      supportedInterfaces: ['USB', 'ESP8266', 'Telnet'],
      ports: portsList,
      activeInterface: "",
      activePort: "" // or activeIP in the case of wifi/telnet?
    },
  },
  logEntry: "String" // In electron we lose the cmd log - can we also emit the log lines here?
};

[ghost]

So basically, in server.js, everywhere where there now is an socket.emit, that gets replaced with a updateStatus(key, value) that updates the object, and triggers an emit of the entire object? Or we could emit the object on a loop every couple ms?

[tbfleming]

I like this.

[ghost]

Yay! (:

[ghost]

PS: Feel free to add more data as you guys see fit / need too~!

[tbfleming]

Instead of a single work offset, it'd benefit mill and lathe to have the entire set of offsets: G54, G55, ..., G92, G43, and which work offset is active. The normal UI would hide most of these, but it'd be available to custom SVG UIs.

[tbfleming]

To improve efficiency, might have to just send fields with changed values.

[ghost]

Great! That works for me too! Love where this is going!

[ghost]

changed values is fine too (: - at the end of the day in my frontend i still have a single object to worry about

[tbfleming]

As part of the new control UI, I've been pairing down com.js to only talk to the server and not handle any UI issues. It leaves the UI to UI components.

[cprezzi]

I don't realy see how this one big object could simplify the whole thing. If we pack everything in a single websocket event, the client has to parse the object to find out what to do. This also could lead to a performance issue.

I think it's very convenient and easy to attach functions to specific events (like status changes, response to commands, error events) to handle specific tasks. Sure, there is room for cleanup all the events, for example group together all position or status changes, but I wouln'd pack everything in one single ws event.

Or did I understand you wrong?

[tbfleming]

There's exactly 1 event in the react-redux world for communicating system state changes: the store changed, triggered by dispatch(). It lets us pretend that the component tree regenerates the entire DOM from scratch whether a single data item changed, or whether there are massive changes. It greatly simplifies updating the UI; there's no "if state changed from X to Y" code that can miss cases. There are optimizations that greatly reduce the cost, but they rely on diffing immutable objects using identity, not handling separate events.

[tbfleming]

The updated com.js translates between the V1 protocol and the react-redux way of doing things. It's loaded with code like this that I could axe if we switch to @openhardwarecoza 's proposal:

        socket.on('serverConfig', data => {
            this.setComAttrs({ serverConnecting: false, serverConnected: true });
            let serverVersion = data.serverVersion;
            this.setComAttrs({ serverVersion: serverVersion });
        });

        socket.on('interfaces', data => {
            this.setComAttrs({ serverConnecting: false, serverConnected: true });
            if (data.length > 0) {
                let interfaces = new Array();
                for (let i = 0; i < data.length; i++)
                    interfaces.push(data[i]);
                this.setComAttrs({ interfaces: interfaces });
            }
        });

        socket.on('ports', data => {
            this.setComAttrs({ serverConnecting: false, serverConnected: true });
            if (data.length > 0) {
                let ports = new Array();
                for (let i = 0; i < data.length; i++) {
                    ports.push(data[i].comName);
                }
                this.setComAttrs({ ports: ports });
            }
        });

[ghost]

And for me, still to lazy to learn React, the same thing works nicely with Jquery http://www.lucaongaro.eu/blog/2012/12/02/easy-two-way-data-binding-in-javascript/

Right now i also pipe the various "events" into populating that object i want anyway, and from there its go time

[ghost]

Other advantage: To get more diverse frontends to maybe try out comm-server (focus being on making it even less LW connected and more its own thing) - the need to document the API becomes a lot less, since there is only one status event to subscribe to.

Conversely we could probably make the server side more efficient eventually too by sending the UI side events to it in a similar fashion

[ghost]

Last advantage i just also though of - it cuts out the need for that firstRun even - if a new client connects the first event it gets already tells it EVERYTHING it needs to know (;

[tbfleming]

This probably deserves a different issue: it'd be nice to dump the socket IO library we're using; it's strict client-server version check has been a maintenance nightmare on the packaging side. They keep promising better webpack support with each release, but they never fix the fundamental tension between their model and package managers (all of them). e.g. pitfall: disconnecting a client app from 1 server (e.g. running a lathe) and connecting it to a different server (e.g. running a laser) when the 2 are running different socket library versions.

[ghost]

Yes, especially when the second client is hosted from its own server. I also started an Android port a while back - where i wanted the frontend to WS to the server - couldn't even find a socket.io lib for android that worked (went with Webview for now because of that.)

On Aug 11, 2017 3:45 PM, "Todd Fleming" notifications@github.com wrote:

This probably deserves a different issue: it'd be nice to dump the socket IO library we're using; it's strict client-server version check has been a maintenance nightmare on the packaging side. They keep promising better webpack support with each release, but they never fix the fundamental tension between their model and package managers (all of them). e.g. pitfall: disconnecting a client app from 1 server (e.g. running a lathe) and connecting it to a different server (e.g. running a laser) when the 2 are running different socket library versions.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LaserWeb/lw.comm-server/issues/47#issuecomment-321815769, or mute the thread https://github.com/notifications/unsubscribe-auth/AHVr250FThWNbqcKAs643QNc4dYlqkVEks5sXFrggaJpZM4OzhhO .

[cprezzi]

But could we make sure that unchanged status is not sent again and again (for performance reason)?

And what about multiple client connections? I've planned to send the actual job's gcode back to a new client if there is a job already running. This way the second client could show the workspace even if the gcode was not sent from him.

[cprezzi]

@openhardwarecoza You should not need a specific socket.io library to talk to the node socket.io port, it should work with other websocket implementations too. At least I was able to talk to a node websocket from a php app.

[jorgerobles]

When socket is first time inited, it could take a the JSON map the client needs. IE: http://server:8080/endpoint.json?fields=x,y,z,or even send a JSON structure if query fields is not enough.

{
    sendmeX:true,
    sendmeY:false
}

I usually to that kind of things in my regular job :P

[cprezzi]

@jorgerobles I like that approach. The client tells the server at beginning what info/features he likes to subscribe.

[cprezzi]

But first, the server should tell the client what info/features he has to offer (+server version, protocoll version...) ;)

[jorgerobles]

Well that's an initial contract. Once setup will be fine

[tbfleming]

Or, it could send everything the first time and only changed fields after that point. That's much simpler for my code to deal with, and I suspect @openhardwarecoza 's also.

[tbfleming]

You could embed a version into that initial object. I'll ignore it. Extra fields I don't yet understand won't hurt me, just like new fields appearing in reducer definitions won't hurt me.

[ghost]

Yeah, send everything initially. Update only what changes. Still efficient enough. Frontend then just uses what it wants to use.

On Aug 11, 2017 4:27 PM, "Todd Fleming" notifications@github.com wrote:

Or, it could send everything the first time and only changed fields after that point. That's much simpler for my code to deal with, and I suspect @openhardwarecoza https://github.com/openhardwarecoza 's also.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LaserWeb/lw.comm-server/issues/47#issuecomment-321828380, or mute the thread https://github.com/notifications/unsubscribe-auth/AHVr22WK_KdHLH5hPfY5CTRAgBp53CJYks5sXGTWgaJpZM4OzhhO .

[cprezzi]

That would work, but could mean we send too much data during the whole session, that the client doesn't need and just create network traffic (bad especially on WiFi).

[cprezzi]

But at least a point to start with.

[ghost]

But is it really that much? As is the headers are 80% of the packet (;

On Aug 11, 2017 4:34 PM, "Claudio Prezzi" notifications@github.com wrote:

That would work, but could mean we send to much data during the whole session, that the client doesn't need and just create network traffic (bad especially on WiFi).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LaserWeb/lw.comm-server/issues/47#issuecomment-321830029, or mute the thread https://github.com/notifications/unsubscribe-auth/AHVr21Qp_q2-vmsFOzoJATONuzNtSNZmks5sXGY6gaJpZM4OzhhO .

[cprezzi]

We will see.

[tbfleming]

Here's something that would benefit my code also:

• Send a flat object, not a hierarchical one. You could use hierarchical names, e.g. positionWorkX or position-work-x
• My code will be blissfully unaware of most of the fields, but...
• SVG files may reference any field you send

[ghost]

Thats a valid point, easier to reference, and no nested objects to deal with... I wouldnt mind, the svg advantage is something i felt hard when i made that CAM diagram (; clearly named svgs fields to show/hide/update is a win. Ps. That svg idea is brilliant!

On Aug 11, 2017 4:37 PM, "Todd Fleming" notifications@github.com wrote:

Here's something that would benefit my code also:

• Send a flat object, not a hierarchical one. You could use hierarchical names, e.g. positionWorkX or position-work-x
• My code will be blissfully unaware of most of the fields, but...
• SVG files may reference any field you send

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LaserWeb/lw.comm-server/issues/47#issuecomment-321830926, or mute the thread https://github.com/notifications/unsubscribe-auth/AHVr20iQ9bGkSxQ-yc0o5GE6K7ea0yRkks5sXGcIgaJpZM4OzhhO .

[cprezzi]

I suggest _ for hierarchy separator, so position_work-x is position->work-x.

[cprezzi]

Oh wait, isn't _ vorbidden in a css class name?

[ghost]

Ps: great job everyone on just talking this through. This is way more valuable than me jumping in and just 'doing'! (:

On Aug 11, 2017 4:40 PM, "Claudio Prezzi" notifications@github.com wrote:

I suggest _ for hierarchy separator, so position_work-x is position->work-x.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LaserWeb/lw.comm-server/issues/47#issuecomment-321831971, or mute the thread https://github.com/notifications/unsubscribe-auth/AHVr22s5DubHELzhu_-BDzZDeYfQWVOjks5sXGfzgaJpZM4OzhhO .

[tbfleming]

I don't remember, but it creates a problem: is work x hierarchical, or non-hierarchical? work-x or work_x? If we always use - then we don't have to solve that.

[cprezzi]

How should we move forward with this? Create a new branch? Or a new repo?

[tbfleming]

I'm ok with either for lw.comm-server. I don't want to create a separate repo on the LW4 side, so it will be on a branch that eventually merges back into dev-es6.

[cprezzi]

Ok. I'll create a new branch for now. Later I probably create a new repo without "lw." to show it's not LW only.

[ghost]

For that repo will also add some better build scripts to pack electron with anyones own UI

On Aug 11, 2017 4:53 PM, "Claudio Prezzi" notifications@github.com wrote:

Ok. I'll create a new branch for now. Later I probably create a new repo without "lw." to show it's not LW only.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LaserWeb/lw.comm-server/issues/47#issuecomment-321835325, or mute the thread https://github.com/notifications/unsubscribe-auth/AHVr2-FqauV32bOrOu_m6ZSrYXl9CPzcks5sXGregaJpZM4OzhhO .

[tbfleming]

The electron build script already supports that. You just have to rename your app's directory to "LaserWeb4" :)

[ghost]

(: lol!

On Aug 11, 2017 5:08 PM, "Todd Fleming" notifications@github.com wrote:

The electron build script already supports that. You just have to rename your app's directory to "LaserWeb4" :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LaserWeb/lw.comm-server/issues/47#issuecomment-321839316, or mute the thread https://github.com/notifications/unsubscribe-auth/AHVr2-o3ytzJxyIuTqFmVxFyPspviztcks5sXG5ogaJpZM4OzhhO .

[cprezzi]

The easyest would be that frontend files are just placed into the app folder and we make sure electron uses app/index.html as the frontend.

[ghost]

I was thinking env variables (apppath=...) instead, and using nodejs instead of cli to handle the build, helps with naming files with version, etc from the word go

On Aug 11, 2017 5:29 PM, "Claudio Prezzi" notifications@github.com wrote:

The easyest would be that frontend files are just placed into the app folder and we make sure electron uses app/index.html as the frontend.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LaserWeb/lw.comm-server/issues/47#issuecomment-321844586, or mute the thread https://github.com/notifications/unsubscribe-auth/AHVr244XtB85iY0wBFdS-wTjrph7djHgks5sXHNjgaJpZM4OzhhO .

[tbfleming]

The drawable UI is getting closer to rollout, but I'm thinking of waiting for this since all the attrs exposed to the SVG files will change.

[cprezzi]

I'm not sure if it makes sense to wait. I'm very busy in my dayjob at the moment and don't have enough time to do the conversion.