Telnet / node.output example not working

[SBartley08]

Expected behavior

when using the example to redirect node input and output I expected from the docs that all output would be redirected

Actual behavior

if syntax is correct everything works ok but if there is an error for example 1+1 not print(1+1) all that is returned is >

however lua does output the correct error on UART
stdin:1: unexpected symbol near '1'

Test code

-- a simple telnet server
s=net.createServer(net.TCP)
s:listen(2323,function(c)
   con_std = c
   function s_output(str)
      if(con_std~=nil)
         then con_std:send(str)
      end
   end
   node.output(s_output, 0)   -- re-direct output to function s_ouput.
   c:on("receive",function(c,l)
      node.input(l)           -- works like pcall(loadstring(l)) but support multiple separate line
   end)
   c:on("disconnection",function(c)
      con_std = nil
      node.output(nil)        -- un-regist the redirect output function, output goes to serial
   end)
end)

NodeMCU version

NodeMCU custom build by frightanic.com
branch: master
commit: 22e1adc
SSL: true
modules: file,gpio,mqtt,net,node,tmr,uart,wifi,ws2812,tls
build built on: 2017-04-15 15:37
powered by Lua 5.1.4 on SDK 2.0.0(656edbf)

Hardware

NodeMCU Dev Board

[jmattsson]

I think the work you're doing on Lua 5.3 would touch on this, @TerryE.

[TerryE]

Yes it does!

[mdeneen]

Can you expand on this a little further, @TerryE ?

[TerryE]

@mdeneen, Mark, please read #1661 and the referenced whitepaper for detailed discussions. Sorry for it being a long read, but it is a bit of a mammoth task.

[mdeneen]

@TerryE Looks like quite the undertaking. I thought that the output to uart0 instead of the socket had something to do with using 5.3, but it's more to do with the overall Lua interpreter integration.

[TerryE]

Back to the OP, this example in the node module documentation won't work after the 0.9x releases as you can't issue multiple sends. Here is my version that does:

-- a simple telnet server
wifi.setmode(wifi.STATION, false)
wifi.sta.config { ssid = "YourSID", pwd  = "YourPWD", save = false }
tmr.alarm(0, 500, tmr.ALARM_AUTO, function()
--  uart.write(0,tostring(wifi.sta.status()) )
  if (wifi.sta.status() ~= wifi.STA_GOTIP) then return end
  tmr.unregister(0)
  print(wifi.sta.getip())
  net.createServer(net.TCP, 180):listen(2323, function(skt)
    local push, unshift = table.insert, table.remove
    local sending, fifo  = false, {}
    local function sendchk(c)
      if #fifo == 0 then sending = false; return; end
      c:send(unshift(fifo)); sending = true
    end
    
    skt:on("receive",       function(c, l) node.inpu(l) end)
    skt:on("disconnection", function(c) node.output(nil) end)
    skt:on("sent",          sendchk)
    
    node.output(function(str) push(fifo, str); 
                  uart.write(0,str,'\n')
                  if not sending then sendchk(skt) end; 
                end, 0)
    print("Welcome to NodeMCU world.")
  end)
end)

But even this has issues as the commands are executed through lua.c:dojob() with the stdout stuff being redirected to the output function. However, if the statement is invalid, the error is reported by a report function which then calls lua.c:l_message() and this outputs directly the the UART, so the stderr stuff doesn't get redirected.

There's also something bizarre going on with the ordering, but I'll be bottoming this in other testing that I'll be doing.

[mode2k]

any news on redirecting stderr to uart1 or to a socket?

[TerryE]

It's on my TODO list after we have the LFS patch in dev.

[TerryE]

I have updated my telnet example in this gist. I will be adding it to LFS as soon as I've tracked down a subtle issue that this has thrown up with LFS node.input() processing. Unlike the previous telnet which where you could only output small amounts per Lua statement, this one buffers and outputs large outputs fairly robustly.

[marcelstoer]

New Telnet module has landed, #2416 -> closing

[HHHartmann]

@marcelstoer This issue is about redirecting error output to node.output() which is not fixed yet.
Please reopen

[jmattsson]

Wasn't that fixed in the LFS merge a few weeks ago?

[HHHartmann]

hm can't test right now because of #2430
but reasoning from @TerryE s comment above it is not

It's on my TODO list after we have the LFS patch in dev.

[HHHartmann]

Just tried with Terrys telnet and issues a comman over serial.
`` print("was") dofile("none")

It prints

"was"

on the telnet session and the error message about the not found file on the serial

print("was") dofile("none")
cannot open none
stack traceback:
[C]: in function 'dofile'
stdin:1: in main chunk

So this is not fixed

@marcelstoer please reopen this issue

[jmattsson]

Paging @TerryE on this one then :)

[TerryE]

Actually thinking about this, this is the same as #2430. The underlying issue in OPs first comment is nothing to do this @HHHartmann Gregor's issue. Let's only track one open issue on this.

[TerryE]

@HHHartmann Gregor, I should have considered this properly on first reading, but having done so I feal that the way that this issue is framed is as if the firmware implements a single unified POSIX-style handling of stdout and stderr FDs. Unfortunately it doesn't.

• Calls to the print function are handled in a single piece of code in lbaselib.c which invokes c_puts() which is in turn a define to node.c:output_redirect().

• Error handling is scattered throughout the Lua code. For example interactive compilation errors are handled in lua.c:l_message() that calls luai_writestringerror() which is a define for dbg_printf.c:dbg_printf(), and this by design directly sends to the UART so that error messages get posted when debugging. Redirecting this might be possible but this could also have other side effects that we would need to consider.

• Non-interactive errors are handled completely different in that they have to honour the Lua protected call architecture. With this, the error is either caught and handled in the application or it is not and the error results in a panic where by definition this has totally unrolled the stack, so the concept of invoking a Lua call back at the stage makes no sense.

Long story short: this might seem a simple change but it isn't; it is an architectural change that would require careful thought, and therefore falls clearly into an enhancement request category, and one that I am not prepared to take on at the moment. I have other priorities.

[HHHartmann]

@TerryE Terry thanks for reopening this issue. Seems as if this will be a collection of fixes then.
Maybe in the meantime I will just connect rx and tx on the serial and add the serial input to the stream of outputs from node.output() then.

ok so this

[TerryE]

I think that the mistake here is to treat the telnet daemon as a normal console session or even the exact analogue of the UART serial interface. If you look at what @nwf Nathaniel has done, he has in effect added a simple command interpreter. If you do this then you can have an immediate Lua command which the CI wraps in a pcall so that it captures the error output using standard Lua functionality, and the whole stderr issue and panic goes away. You can also add commands like dir, type, del, upload, etc.

With the support of a python script on the host, you can then do these immediately at the cmd prompt on your pc, e.g.

esp upload myLFS.img
esp load myLFS.img
esp dir
esp lua for k,v in pairs(_G) do print(k,v) end

etc.

If you go this way then the sensible default is to close the TCP connection after each command unless you enter the bare esp command on work from the esp> prompt host-side.

[stale]

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.

[HHHartmann]

This might be fixed in the current dev branch. Will try these days

[TerryE]

It should be fixed after #2836 has been merged, but that will be after the current master drop has been merged into master.

[HHHartmann]

Terry I knew you had fixed it but I somehow lost track on which branch it was.

[TerryE]

Fixed in #2836

[HHHartmann]

This might be fixed in the current dev branch. Will try these days

Ok "these days" were some more days and now I tried.

The aggregating of the command before executing it does not work yet. Each call to node.input is executed immediately. So the above example (and that of out samples section) does not work.

Currently this works:

tmr.create():alarm(1000, tmr.ALARM_SINGLE ,function() node.input("print(1223)") print('sent input')  end) 

but this doesn't

tmr.create():alarm(1000, tmr.ALARM_SINGLE ,function() node.input("print(12") node.input("23)") print('sent input')  end) 

[TerryE]

@HHHartmann Gregor, that's because the input processor only processes complete lines, so you need to emit a \n for the interpreter to receive the line. So this works fine. for example:

local ni=node.input tmr.create():alarm(1000, tmr.ALARM_SINGLE ,function() ni 'print(12'; ni '23)\n' end)

and because of the way input processing works -- that is there is only one stdin pipe which can be fed by node.input or the uart, you can even complete the line interactively:

> tmr.create():alarm(1000, tmr.ALARM_SINGLE ,function() node.input 'print(12345' end)
> 6)
123456
>

So this isn't a bug; it's the way its designed to work. This just makes telnet more robust.

[TerryE]

As to telnet itself, what we've got here is a backwards compatibility break that was documented in the node documentation but we've still got some documentation tidy up to do. See the node.input() documentation. The argument to output is now a pipe to make record marshalling automatic and the pipe function must return false to stop a retry firing race. The new telnet example works as does this simple version:

function telnet_session(socket)
  local function output_CB(opipe)
    local rec = opipe:read(1400)
    if rec and #rec>0 then
      socket:send(rec, #rec==1400 and output_CB or type) 
    end
    return false
  end  
  node.output(output_CB, 0)
  socket:on("receive", function(_,rec) node.input(rec) end)
  socket:on("disconnection", function(skt) node.output() end)
end
net.createServer(net.TCP, 180):listen(2323, telnet_session)

The use of type() is just a botch because soc:send() only updates the sent CB if parameter 2 is a function. It should really accept a boolean false to remove it.

[TerryE]

@HHHartmann, perhaps you could review the documentation or decide to close this, because there isn't any bug per se here, just a tweak to the documentation needed.

[HHHartmann]

@TerryE Terry, sorry for the hassle, I seem to have mixed up my devices and tested on an old one. With the current dev build everything works fine.
Thanks for your great work.