ESP32: Websocket client ESP32 library in pure Lua #2694

chilipeppr · 2019-03-14T21:13:00Z

This is a library for a websocket client for ESP32 written in pure Lua. This is a completely license-free library I wrote on my own as my contribution back to the nodemcu community.

I'd like to add something to the DOCS as well on this library since I know I go to https://nodemcu.readthedocs.io/en/dev-esp32/ all the time and look for existing libraries, however, none of the pure Lua libraries seem to be in those docs. I'd love to get some in there though because I always forget which Lua libraries are part of the nodemcu ESP32 repo.

This PR is for the dev-esp32 branch rather than for master.
This PR is compliant with the other contributing guidelines as well (if not, please describe why).
I have thoroughly tested my contribution.
The code changes are reflected in the documentation at docs/en/*.

The rationale for this PR is that the NodeMCU ESP32 framework needs a websocket client library.

This is a library for a websocket client for ESP32 written in pure Lua. This is a completely license-free library I wrote on my own as my contribution back to the nodemcu community.

HHHartmann

Great contribution. So now there will be websocket for esp32 also

HHHartmann · 2019-03-15T05:48:37Z

lua_examples/websocket.lua

@@ -0,0 +1,49 @@
+-- ChiliPeppr Websocket client library for Lua ESP32


Usually lua_module examples are right besides the module in the same folder.
See http module for example,

Hmm, I did it like the email module which had a folder in examples and a folder in lua_modules. I can do it like http though if that's the more preferred format.

ok checked some others and both ways seem to be common.
So a task to decide for Collaborator (or not to decide)

I moved them to the same directory. I think it's fine that way.

HHHartmann · 2019-03-15T05:48:45Z

lua_examples/websocket.lua

+end)
+
+-- Use ChiliPeppr wifi library to auto-connect to wifi
+wf = require("esp32_wifi")


the example maybe should not require external modules. Does it also work with the standard wifi module?

Yeah, it just needs normal connectivity. I should remove that.

lua_modules/websocket/websocket.lua

HHHartmann · 2019-03-15T05:55:33Z

lua_modules/websocket/websocket.lua

+    -- print("Got receive: " .. c)
+
+    -- see if we are good to go here 
+    if isHdrRecvd == false then 


What if the header is not sent in a single packet?
What if data is sent in the same packet as the header?
What if no 101 status is sent?

It obviously works against the server you use, but might not work against other ws servers.

Yes, I suppose that scenario could happen. I wouldn't have the time to go test that though. It still may be worth ignoring that potential scenario to get this websocket.lua into this repo and then allow others to tweak from there if they see that as an issue as a bug later on. The notion is that the header comes in first, so I think probability is high that the first TCP packet is going to have more than those needed 20 chars or so.

probably valid for that case. But what about the other ones?

From reading the standard docs again, your other scenarios would mean the server is not following the websocket standard. I think we're good here.

As I read it the websocket standard just defines logical websocket packets.
The underlying tcp packet size and fragmentation may be different.

The relevant part in the RFC seems to be

After a successful handshake, clients and servers transfer data back
and forth in conceptual units referred to in this specification as
"messages". On the wire, a message is composed of one or more
frames. The WebSocket message does not necessarily correspond to a
particular network layer framing, as a fragmented message may be
coalesced or split by an intermediary.

so two fragments might also be coalesced to one frame.

lua_modules/websocket/websocket.lua

HHHartmann · 2019-03-15T06:09:45Z

lua_modules/websocket/websocket.lua

+  -- print("Our own count of len:", string.len(data))
+
+  -- calc m.lenExpected for next time back into this method
+  m.lenExpected = m.lenExpected - string.len(data)


What if data also contains (the start of) the next message?
Then string.len(data) would be greater than lenExpected and only the lenExpected first bytes would have to be handled here.

Hmm, I feel like that's against the websocket standard, but I'm not totally sure. In my testing that never occurred and I use it so much in the real world I've never run into that bug. I don't think I'd have time to test that further. I still think it may be worth ignoring this item, but up to you.

I really don't know if this happens. Just saw that for MQTT it was needed to cover also this case.
See #2571

My other note would also apply here of corse.
Sorry for being so insistent about this. Just bad if things work sometimes and sometimes not.
But maybe a message would be enough as a first step.
Its not up to me to decide after all.

HHHartmann · 2019-03-15T06:10:40Z

lua_modules/websocket/websocket.lua

+    print("Websocket expecting " .. m.lenExpected .. " more chars of data")
+  else 
+    -- done with data. do callback 
+    m.lenExpected = 0 


This should only be necessary in the case mentioned above

In practice, what I've seen is you get lots of "receive" events from the TCP connection on the incoming data and you don't get all the data, however, you know how much you're going to get due to the frame header. So I'm only trying to solve for the multiple function calls that occur until you get the whole amount of data. Then I give you the callback. I don't think the scenario of getting more data for the next frame is part of the standard. So I'm pretty sure I'm safe here.

I think this has nothing to do with the standard. Its only about packaging the information in one or more packages.
Still might be safe in most cases. Maybe a warning would be nice at first, so in case it happens the user can spot it esily.
But I am not the one to decide what goes in anyways.

I think you're thinking of m.lenExpected in a different way than how it's being used. But, it's my fault for printing this message out. I just commented out that line since it really was just a debug line during development so I could follow what was happening. Check out the new commit. I think this cleans this part up.

Added error statements. Moved websocket_test.lua to same directory. Removed test code that relied on 3rd party libraries.

…mware into dev-esp32

chilipeppr · 2019-03-15T07:30:19Z

Ok, made the changes based on your feedback. I think this is good to go.

For what it's worth, if you check out this Youtube video of the websocket client in action:
https://www.youtube.com/watch?v=ITgh5epyPRk

I'm sending tons of data back and forth over this websocket to jog my CNC machine around, so it pushes that websocket code to the limit and it works perfectly.

chilipeppr · 2019-03-15T17:51:51Z

Great contribution. So now there will be websocket for esp32 also

Do you think there is a good way to add to the https://nodemcu.readthedocs.io/en/dev-esp32/ for a Lua module? I've never seen anybody do that yet, so it gets lost to folks that these pure Lua modules even exist, yet they're super valuable to the community.

marcelstoer · 2019-03-15T21:33:20Z

I'd like to add something to the DOCS as well on this library since I know I go to https://nodemcu.readthedocs.io/en/dev-esp32/ all the time and look for existing libraries, however, none of the pure Lua libraries seem to be in those docs. I'd love to get some in there though because I always forget which Lua libraries are part of the nodemcu ESP32 repo.

Great, yes we need this. @galjonsfigur did a great job reorganizing this for the dev branch into Lua and C modules (see https://github.com/nodemcu/nodemcu-firmware/blob/dev/mkdocs.yml#L40). This will soon land on master with #2695.
Every Lua module folder has a README that points to the documentation and the respective Lua module documentation. See https://github.com/nodemcu/nodemcu-firmware/tree/dev/lua_modules/email for an example.

Can you help us get there for the ESP32 branch?

marcelstoer · 2019-03-15T21:36:13Z

My only concern here is that of consistency and re-use between ESP8266 and ESP32. For the older chip we have had a C module for websockets for over two years.

Can or is this Lua module generic enough to be usable on both? If so, should we retire the C module?

chilipeppr · 2019-03-15T21:59:54Z

This is generic enough, but I would say it does not support long packet length and I think the C version does.

…

On Fri, Mar 15, 2019 at 2:36 PM Marcel Stör ***@***.***> wrote: My only concern here is that of consistency and re-use between ESP8266 and ESP32. For the older chip we have had a C module for websockets <https://nodemcu.readthedocs.io/en/dev/modules/websocket/> for over two years. Can or is this Lua module generic enough to be usable on both? If so, should we retire the C module? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2694 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHidbaZ4AQ0isD7dQxT73Tvi4EUr0FOkks5vXBJVgaJpZM4b1Om_> .

jmd13391 · 2019-03-15T23:33:50Z

Oh PLEASE, do not retire the C module.

nwf · 2019-03-16T19:24:58Z

@jmd13391 Say why?

HHHartmann · 2019-03-16T20:03:18Z

At a quick glance the c implementation does handle fragmentation and coalescation in a robust way.

chilipeppr · 2019-03-16T20:05:18Z

Hmm. It might be worth just cancelling this pull request then. I've struggled with not having a websocket client on ESP32, but it's probably nicer to port the C library. That's beyond what I would have time to do though, so maybe we just punt on this becoming part of the dev-esp32 repo.

HHHartmann · 2019-03-22T14:38:49Z

Maybe it still could serve as an interim solution until the C module gets available.

HHHartmann · 2019-07-21T18:49:30Z

this would fix #2186

rushim1 · 2020-03-22T10:05:26Z

hi, Just curious when this will get merged into the dev-esp32 branch.
Thanks.

chilipeppr · 2020-03-22T15:35:23Z

It might be better for this to just be put in the Lua modules directory at https://github.com/nodemcu/nodemcu-firmware/wiki/Lua-modules-directory

marcelstoer · 2020-03-22T20:54:19Z

@chilipeppr I wouldn't mind hosting modules provided (and maintained) but committers in the repo 😉 I think we all lost track of whether reviewer @HHHartmann was happy to approve this or not.

HHHartmann · 2020-03-23T21:42:16Z

As stated above:

At a quick glance the c implementation does handle fragmentation and coalescation in a robust way.

So it might be that this Implementation will not work with all server/network combinations because the messages might not be aligned with the packets being sent,
But it is still better than nothing as it works at least for @chilipeppr and probably in many other places.

So my conclusion was

Maybe it still could serve as an interim solution until the C module gets available.

HHHartmann · 2020-04-10T08:47:12Z

Maybe this could also be the first entry in https://github.com/nodemcu/nodemcu-firmware/wiki/Lua-modules-directory

jpeletier · 2020-05-11T08:13:41Z

I think this is the solution. We should close this PR and move all Lua modules to this wiki.

I'm advocating the same also for C modules. There are lots of PRs here, including my own, asking the maintainers to merge drivers for their hardware.

For C modules I am advocating for this: #3100, then we can host a registry of modules, similar to npm as a next step, liberating the main repo of the burden of hosting everything, while allowing permissionless creation of new modules.

marcelstoer · 2020-05-11T08:21:06Z

We should close this PR and move all Lua modules to this wiki.

See #3034. The intention is to host "endorsed" Lua modules here and use the wiki as a registry for others. What endorsed means has yet to be defined, though. We were discussing to require at least unit tests.

jpeletier · 2020-05-11T10:49:23Z

I wouldn't make a distinction between endorsed and not endorsed. If useful, the community will endorse them via repository stars, contributions, etc.

I would move all existing lua modules out and actually use that move to kickstart the wiki with some quality content that encourage people to add their stuff there as well.

If you guys give me some org access, I can do this work: for existing modules, I would create a repo for each, move the code there along with its git history and link those in the wiki. That way, each module can have its own space for PRs and contributions.

For new modules, people can just go create their own repo and link it in the wiki themselves.

pjsg · 2020-05-11T14:14:10Z

For the C modules, I am against moving them out. The reason is that the modules in the nodemcu-firmware repo are supposed to work, we keep them running as the underlying lua vm moves forward. The difficulty is the level of review of changes to these external modules. They won't get the attention from the core maintainers, and when they stop working, it is likely that the users will complain to nodemcu-firmware rather than this other repo.....

jpeletier · 2020-05-11T14:49:58Z

For the C modules, I am against moving them out. The reason is that the modules in the nodemcu-firmware repo are supposed to work, we keep them running as the underlying lua vm moves forward.

For the existing C modules, I am proposing they move to a separate repository but within the nodemcu GitHub org, with this criteria:

Modules that expose internal ESP hardware or are core software libraries anyone uses, such as http, json, mqtt, etc. These can be tested by anyone with a dev board: STAY as is.

Other modules already there that do not meet the above criteria: move to a new repo within the nodemcu org. Example: dht, bme280...

New modules from now on that drive external hardware: Owner's repo as proposed in #3100

The difficulty is the level of review of changes to these external modules. They won't get the attention from the core maintainers, and when they stop working, it is likely that the users will complain to nodemcu-firmware rather than this other repo.....

As exposed above, these moved modules would stay within the org and we can automate tests so we know they still compile.

In any case, we can't endlessly drag forward modules we don't even know whether they are used or not. We can't sustain a huge centralized and monolithic repo forever and centralizing reviews in a few people.

stale · 2021-06-02T16:55:02Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Websocket ESP32 library in pure Lua

b112528

This is a library for a websocket client for ESP32 written in pure Lua. This is a completely license-free library I wrote on my own as my contribution back to the nodemcu community.

chilipeppr changed the title ~~Websocket ESP32 library in pure Lua~~ ESP32: Websocket ESP32 library in pure Lua Mar 14, 2019

chilipeppr changed the title ~~ESP32: Websocket ESP32 library in pure Lua~~ ESP32: Websocket client ESP32 library in pure Lua Mar 14, 2019

HHHartmann requested changes Mar 15, 2019

View reviewed changes

chilipeppr added 4 commits March 15, 2019 00:16

Fixed items from pull request feedback

06d0c90

Added error statements. Moved websocket_test.lua to same directory. Removed test code that relied on 3rd party libraries.

Delete websocket.lua

531bbba

Moved test file to correct directory

772dd57

Merge branch 'dev-esp32' of https://github.com/chilipeppr/nodemcu-fir…

c631743

…mware into dev-esp32

Remove telling user more chars of data are expected

52d0a20

marcelstoer added the ESP32 label Mar 15, 2019

marcelstoer mentioned this pull request Mar 22, 2020

Reorganize docs for dev-esp32 #3059

Open

stale bot added the stale label Jun 2, 2021

stale bot closed this Jun 16, 2021

		@@ -0,0 +1,49 @@
		-- ChiliPeppr Websocket client library for Lua ESP32

ESP32: Websocket client ESP32 library in pure Lua #2694

ESP32: Websocket client ESP32 library in pure Lua #2694

Conversation

chilipeppr commented Mar 14, 2019 • edited by marcelstoer Loading

HHHartmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chilipeppr commented Mar 15, 2019

chilipeppr commented Mar 15, 2019

marcelstoer commented Mar 15, 2019

marcelstoer commented Mar 15, 2019

chilipeppr commented Mar 15, 2019 via email

jmd13391 commented Mar 15, 2019

nwf commented Mar 16, 2019

HHHartmann commented Mar 16, 2019

chilipeppr commented Mar 16, 2019

HHHartmann commented Mar 22, 2019

HHHartmann commented Jul 21, 2019

rushim1 commented Mar 22, 2020

chilipeppr commented Mar 22, 2020

marcelstoer commented Mar 22, 2020

HHHartmann commented Mar 23, 2020

HHHartmann commented Apr 10, 2020

jpeletier commented May 11, 2020

marcelstoer commented May 11, 2020

jpeletier commented May 11, 2020

pjsg commented May 11, 2020

jpeletier commented May 11, 2020

stale bot commented Jun 2, 2021

chilipeppr commented Mar 14, 2019 •

edited by marcelstoer

Loading