Feature Websocket module

[luismfonseca]

Fixes #1425.

• This PR is compliant with the contributing guidelines (if not, please describe why).
• I have thoroughly tested my contribution.
• The code changes are reflected in the documentation at docs/en/*.

Websockets allow a two-way communication with servers which is better suited for realtime applications. Existing Lua impementations are not as fast and take over 10KB of heap for a single connection.

Since it has been a several years without programming in C or C++, I'm sure my code could be improved. Please let me know where. Also, I'd like some additional testing to other websockets servers (I've mainly tested against Pushers and websocket.org).

Committers supporting this PR: .

[luismfonseca]

Hi, I know you guys are busy with the new releases and whatnot but I'd like to know what I can do to progress this further. (Or is this just rejected?)

Btw, for reference sake, this solves issues #30 and #384.

[devsaurus]

I will review the code for formal aspects, but websockets is not a use case for me. We'd need someone else who can assess the overall design and fitness for typical use.

[luismfonseca]

Ok, thank you.
Perhaps @creationix and @ikaras can provide input as well? (Seeing that both actually implemented a websocket interface).

[devsaurus]

All checks for ws after luaL_checkudata() are redundant since this function will effectively not return to your context in case of an error. It'll long jump to the calling Lua context instead. Ref. https://www.lua.org/manual/5.1/manual.html#lua_error.
Also applies to similar instances below.

[devsaurus]

As a general remark: your code bases on espconn which will vanish sooner or later. There are already efforts ongoing to re-base networking to LWIP in #1379. Don't know if it's worth considering this here already from the beginning.

[djphoenix]

I really propose to reject new modules that use espconn layer. It's very bad and bugous.
If it uses TLS client mode, wait for #1435 and rebase it on mbedtls.

[creationix]

My use case is websocket server which doesn't appear to be in here.

[luismfonseca]

Thanks for reviewing @devsaurus.

I also feel that espconn layer is bad, so I understand the need to remove it. It would be nice to continue to work on this so it uses LWIP, but on my next MR. That wouldn't make me feel that I wasted so much time battling the espconn!

@creationix: Yes, you're right. But it would be mostly the same except from having a new method: createServer(function(ws)) which would receive a function to handle new connections, the interface would be the same. And maybe another function to close the server.
I only added you to this discussion as you use websockets and nodemcu, and thus could provide an insight about the interfaces (even if it is only client side for now).

[devsaurus]

As mentioned earlier, luaL_unref() handles LUA_NOREF. You can skip the "if" here and below.

[devsaurus]

Apart from cosmetic topics, the final gating question IMO is: shall we accept a new module that's based on espconn?

[luismfonseca]

How far away do you reckon both #1379 and #1435 are?

Like I said, I don't mind doing another MR to implement this using lwIP. I also think it will be better.
It would be nice having this version merged though.

[marcelstoer]

shall we accept a new module that's based on espconn?

👍 from my side if we have the commitment from the author / maintainer to port it to lwIP once that's ready - and we've got that here.

[marcelstoer]

Btw Luís, since you just created your first NodeMCU module (I believe) can I invite you to edit / amend "How to write a C module" if you have any feedback.

[luismfonseca]

Thanks for merging this!

Ok, I'll work on that. But I'd say that as it stands now, it is already helpful.

[luismfonseca]

@marcelstoer I've added a small section for debugging and change the start a tiny bit, feel free to adjust as you see fit.