On Sun, 19 Nov 2017, Mark Webb-Johnson wrote:
Strange. When I tested mongoose, I got a close event (MG_EV_CLOSE) for each open connection, when the interface went down.
It does send a close event, but not until after the wifi is already shut down so closing the socket at that point does not send any packet to the client. I've decided to punt on that issue, though, because manually shutting down wifi is not an important use case. The more likely case is that wifi connectivity is lost due to motion or other causes and in that case no close packet can be delivered anyway. My other points were:
- It looks like there may be a memory blocked leaked at each client disconnection. I need to find that.
This is not a leak. It is the policy of lwip to allocate a semaphore to each socket and to not reuse socket structures in its pool until all have been used. So that means up to 10 semaphore blocks of 92 bytes each (104 with debug overhead) will be allocated as new client connections are made. I actually figured this out once before in August when I first implemented telnet.
- I think I can improve how I handle the receive buffer and save some more RAM.
I've also thought more about how to convert the SSH Console and will take a stab at that as well.
In my local repository I have done both of these improvements but not yet committed them. Furthermore, I have determined that the "surgery" in OvmsConsole required to accommodate ConsoleAsync as a task while ConsoleTelnet and ConsoleSSH run within the NetManTask was not as bad as I thought. So, as you requested, in my local copy I have eliminated all the dedicated tasks for the Telnet and SSH implementations and have that working. However, there are a couple of downsides to using mongoose: 1) Output from other tasks through the OvmsCommandApp::Log() facility will be delayed on average by half the mongoose polling interval, which is currently one second. In my original implementation I avoided polling to avoid delays, but that had other costs. 2) More serious: Mongoose has a policy that callback function will not block, which is reasonable since it acts as a hub for multiple services. However, one consequence is that the mg_send() function always just buffers the data you give it into heap RAM. So, in the SSH console, for example, when you enter a command an MG_EV_RECV event will pass control into code in the console and command classes. That code will generate its output as multiple puts or printf calls each of which results in an mg_send() call. For a command like "?" that does many lines of output, mongoose will be doing a bunch of realloc() operations to keep appending all of the lines into one big buffer in RAM. It is possible for a command to use up all the available RAM. Fortunately mg_send() fails gracefully when that happens, but it means command output is lost and in the meantime other functions that depend upon RAM may fail less gracefully. It was issues like this that led me to be concerned about eliminating the console tasks. There is not any mechanism in Mongoose for the application to flush the send buffer. The data will only be sent when the callback for the MG_EV_RECV event returns to Mongoose. We don't have a way to return to mongoose part way through the command output and then get control back again to finish the command. The state of being in the middle of the command is lost without it being a separate task to hold that state. It might work to recurse into mongoose by calling mg_mgr_poll() again, but the code in mongoose may not be reenterable and this would incur significant risk of stack overflow. Should we consider modifying Mongoose to change its policy so that mg_send() actually sends its data? This could be a flag in the mg_connection object, or there could just be an mg_flush() call added. Digging through the code I found mg_mgr_handle_conn() that does the actual sending, but that function is not exposed in the header file and I don't know whether it would be valid to call it from user code while in the middle of the callback. You also said:
Perhaps a generic extension to ovms_netmanager where you give it a tcp port and an object type. Then, it listens on that port and launches objects of that type (pre-configured to receive mongoose events as appropriate) for each new connection? Have a base virtual connection class, that gets inherited by each implementation (ssh, telnet, etc).
Right now I just have some similar code in OvmsTelnet and OvmsSSH classes. When this settles down we could consider factoring that int a base virtual connection class. -- Steve