nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32
https://nodemcu.readthedocs.io
MIT License
7.64k stars 3.12k forks source link

Multiple conn:send() not working #730

Closed positron96 closed 8 years ago

positron96 commented 8 years ago

When I was on a stable build (0.9.6-dev_20150704), the code like this worked correctly:

    connection:on ("receive",
    function (conn, req)
        conn:send("HTTP/1.0 200 OK\n")
        conn:send("Server: ESP (nodeMCU)\n")
        conn:send("Content-Length: 1\n\n")
        conn:send("a")
        conn:close()
    end )

This worked for a client socker as well as a server socket.

Now I moved to dev build that says:

NodeMCU custom build by frightanic.com
    branch: dev
    commit: d8c6fe0b25142f6ae866c18c3bcb3d0538d4baaf
    SSL: false
    modules: node,file,gpio,wifi,net,pwm,tmr,uart,ow
 build  built on: 2015-11-03 20:47

And only first conn:send actually sends, all following calls get silently discarded. I now need to store the lines into a table and join table into a string which is probably a waste of memory.

Is it a bug or is it intended?

TerryE commented 8 years ago

The outcome of your code is indeterminate as network requests are treated as separate tasks by the SDK. Ignoring the fact that you should marshal your sends where practical, something like this would work:

  connection:on ("receive",
    function(sck, req)
      local response = {"HTTP/1.0 200 OK", "Server: ESP (nodeMCU)", "Content-Length: 1\n", "a"}
      local function sender (sck)
        if #response>0 then sck:send(table.remove(response,1))
        else sck:close()
        end
      end
      sck:on("sent", sender)
      sender(sck)
    end )

One network call per task and no ordering problems. And yes this is could be viewed as bad coding to have two semantically different variables both called sck but in reality they both point to the same userdata. This really falls under #719 unless this also fails, but I'll give you a chance to reply first :smile:

In reality, I'd never encode it this way because your algo implicitly assumes that you'll get a bare request and this rarely happens. HTTP 1.1 is a complex protocol yet everone using it seems to make a load of dumb assumptions and then wonders why their code is flacky.

jmattsson commented 8 years ago

Is it a bug or is it intended?

This is a result of changes within the SDK. Pre-1.x the SDK seemed willing to buffer up contents from several send() calls despite it not being officially supported, but that is no longer the case. I suspect (but haven't verified) that this happened when they freed up substantial amounts of RAM for application use.

So well, this is neither a bug nor intended, it's simply what's being imposed by the SDK.

positron96 commented 8 years ago

Hello Terry. The code you provided works, as does concatenating the table into string, but sending a table with recursive ''''sent'''' events is not very intuitive to say the least. I'll sacrifice memory and stick with table concatenation as more clear way. Also, could this behavior be documented anywhere? I believe I'm not the only one trying to send data with several successive calls. (Btw, my example code is not the whole piece of http request handler of course:-)

TerryE commented 8 years ago

@positron96 read my Unofficial NodeMCU FAQ. It really need a major update but it does explain this sort of issue. As Johnny says, this was never supposed to work but it did on 0.9x unless you died on memory and PANICed

dwery commented 8 years ago

This seems to be a common issue, it happened to me too as it worked on 0.9.5. maybe the http example or documentation could be amended or a better example provided. I resorted to concatenating a couple of strings but it's not very classy,

nanodeath commented 8 years ago

Even if this is "as intended", it's still confusing as a new user. I've added a new Note section to the docs, which ultimately links back to this page; feel free to update it if it needs it.

makefu commented 8 years ago

@TerryE based on your response i tried to write a bit larger file (> 1024bytes) but i have the issue transferring the file in a chunked manner because file IO is slower than the socket IO.

What is the nodemcu way to solve this issue?

Hellsy22 commented 8 years ago

Why not fix this on firmware side? I mean let's return the queue. Most of languages care about sending data to socket (syswrite, socket.write, print SOCKET etc) - for user it working just like a black box.

TerryE commented 8 years ago

@Hellsy22, Espressif broke backwards compatibility because the 0.9x approach burned memory resources and caused all sorts of instabilities. We don't see it as our job to undo sensible SDK changes. The SDK is task-driven, not procedural. We can't change this.

Hellsy22 commented 8 years ago

"We don't want to burn memory resources by queueing send on SDK side, so we force all users to burn twice more resources by queueing at the LUA side". Ok, I got the point.

But what if I'll make pull-request with a new method called "qsend" working as old fashioned send? Will it be ok or still unaccepable?

TerryE commented 8 years ago

Why bother? Replicating the old functionality will be really complex and memory intensive. Getting it right is going to be difficult. It's just so much easier to code up your Lua correctly in the first place.

Hellsy22 commented 8 years ago

Because simplicity is the one of a greatest NodeMCU's benefits. Last time I checked project page it was "Easy to program". Not "Will teach you how to code up your Lua correctly and then will break reverse compatibility to teach you again and again".

So, will do it anyway. As a lib on Lua side or as a PR for firmware. I prefer the PR, because it's more effective, but if it's unacceptable - just say it, please.

TerryE commented 8 years ago

There are SDK architectural reasons why this will be almost impossible. With the 1.4+ SDK not only can you only issue one send in a task, you can't issue a close either. Even if you implement an I/O queue in your library, you will need to queue all I/O operations. You have no control over the order of tasks firing, so if you do a couple of sends then have a timer alarm to close the socket, then you have no guarantee that the two sends will be scheduled before the alarm delivers the cb to close the socket. So by all means try, or alternatively wait for the ESP32 and RTOS and port eLua to that.

Hellsy22 commented 8 years ago

@TerryE, but why it can be almost impossible? SDK not supporting queue? Ok, it can be created on nodemcu side: just add a few lines of code to net_send to set flag "sending" or put request to queue of flag already set; to net_socket_sent - to callback net_sent if queue not empty or do lua callback if all sent. And a few more to net_close - to wipe the queue. Adding an additional functions like classic "flush", "shutdown" or "flush_and_close" - will be great.

It can be done on Lua side too, but by very ugly way - I have no idea how to override functions from romtable (may be you know how?), so it will be like: myNet.send( socket, data), myNet.close( socket ), myNet.on( socket, name, callback) etc.

So, I do not coding C for an about 15 years, but still can do it. All questions are - are there the better way and will it be accepted?

TerryE commented 8 years ago

Coding up a task driven approach in Lua is straight forward. Feel free to develop what you want, and by all means issue a PR, but: it has to work and be bug free; it must not interfere with correct SDK functionality; it must not create resourcing issues that will make normal operation unstable; there must be a consensus that general users will want this. I personally think it extremely unlikely that you achieve all these goals.

TonyMony commented 8 years ago

Hello everybody, it seems we will not have a nodemcu version that works old way of file sending. In this case I'm wondering is there any working lua example for sending file chunk by chunk with new SDK?

TerryE commented 8 years ago

Just raised #971 to cover this.

bixente57 commented 8 years ago

Hi,

I have two issues trying to implement your solution but for a on:connection event.

Firt issue : I get a "attempt to call field 'remove' (a nil value))" error when calling table.remove. This exact same call is OK in a separate sandbox file. Maybe this is a scope problem.

Second issue (main issue) : this code doesn't work:

conn:on("connection", function(sck, payload)
      local t_req = truncateString(req, 500)
      reverse(t_req)
      local function sender (sck)
        if #t_req>0 then sck:send(t_req[#t_req]) t_req[#t_req]=nil
        else sck:close()
        end
      end
      sck:on("sent", sender)
      sender(sck)
    end)

truncateString returns a table with small packets

I get no http response to this.

The request is OK, I've tried successfully to send it to httpbin.org in one piece. The problem happens only when I try to send in multiple parts.

dvv commented 8 years ago

Well we discuss pure Lua stuff which can be modelled on host computer. The whole example is not seen.

seblanc commented 8 years ago

Dear all, I am looking for a working example to how to read and send multiple html files with the new SDK for a basic webserver. I know this is not the right place to ask, so I opened a question on StackOverflow. Hopefully this could help other people..!

http://stackoverflow.com/questions/36079145/how-to-send-multiple-data-connsend-with-the-new-sdk-nodemcu

RinusW commented 8 years ago

One could solve these issues quite elegantly by using the send(msg, function(cb)) function call in stead of using the :on("send") method. For example, the next code snap (from positron96's first posting )

    connection:on ("receive",
    function (conn, req)
        conn:send("HTTP/1.0 200 OK\n")
        conn:send("Server: ESP (nodeMCU)\n")
        conn:send("Content-Length: 1\n\n")
        conn:send("a")
        conn:close()
    end )

could elegantly reformulated as follows:

   connection:on ("receive",
    function (conn, req)
        conn:send("HTTP/1.0 200 OK\n", function(c1)
          c1:send("Server: ESP (nodeMCU)\n", function(c2)
          c2:send("Content-Length: 1\n\n", function(c3)
          c3:send("a", function(c4)
          c4:close() end) --close c4
          end) --close c3
          end) --close c2
          end) --close c1
    end )

It looks like I'm using function-in-function calls, but these are not normal functions. These are callbacks that get executed when the accompanying send function has been executed. So, the form of writing is function-in-function, but the execution order is completely different. Based on the same principle I also developed a function that sends out a webpage (htlm?) to a webclient. But you will find similar solutions in the above reference to the StackOverflow page.

TerryE commented 8 years ago

@RinusW Rinus, this might be elegant but on a memory-limited system like the ESP, this is an unwise implementation.

pastukhov commented 8 years ago

@TerryE Can you give a short example how it should be?

TerryE commented 8 years ago

OK. I'll bump this on my TODO list, but I want to get #1168 and #1119 out of the way first.

pastukhov commented 8 years ago

Thanks @marcelstoer