[Ovmsdev] Transition to new esp-idf

[Stephen Casner]

Mark,

Well, in turn, I'm sorry for making an API change that was driving you
crazy.  It would have been smarter to add it as a new function even
though that would be duplicating more code.

As the code currently stands, telnet and SSH will work so long as no
operation does more contiguous output than the amount of available
free memory can hold, otherwise an out-of-memory crash will occur.  I
don't know if we consider that an acceptable risk.  Maybe with v3.1
hardware it will be.

Your suggestion to put a new funtion into a separate module is a fine
idea, but that function needs to access some functions in mongoose.c
that are scoped static.  That means we can't entirely avoid modifying
mongoose.

                                                        -- Steve

On Fri, 19 Jan 2018, Mark Webb-Johnson wrote:

> Oops. Sorry. That change broke MQTT. I couldn’t understand what was going on (as mg_send was sending immediately). MQTT works like this:
>
> void mg_mqtt_publish(struct mg_connection *nc, const char *topic,
>                      uint16_t message_id, int flags, const void *data,
>                      size_t len) {
>   size_t old_len = nc->send_mbuf.len;
>
>   uint16_t topic_len = htons((uint16_t) strlen(topic));
>   uint16_t message_id_net = htons(message_id);
>
>   mg_send(nc, &topic_len, 2);
>   mg_send(nc, topic, strlen(topic));
>   if (MG_MQTT_GET_QOS(flags) > 0) {
>     mg_send(nc, &message_id_net, 2);
>   }
>   mg_send(nc, data, len);
>
>   mg_mqtt_prepend_header(nc, MG_MQTT_CMD_PUBLISH, flags,
>                          nc->send_mbuf.len - old_len);
> }
>
> It uses mg_send a bunch of times, then goes back and modifies the send_mbuf by inserting a header, then finishes so that the actual transmission can occur. Seems a really dumb way to do it, but such is life.
>
> It was driving me crazy last night, so in the end I just updated mongoose this morning and hey! everything worked. Now I know why :-(
>
> I see that mg_send_dns_query() does the same (it calls mg_dns_insert_header, which then calls mbuf_insert). Making mg_send transmit immediately would break that as well.
>
> How about introducing a new mg_send_now() that calls mg_send() then sends the data immediately? Perhaps it could be a separate .h/.c file mongoose_extensions to avoid the change getting overwritten?
>
> Regards, Mark.
>
> > On 19 Jan 2018, at 2:36 PM, Stephen Casner <casner at acm.org> wrote:
> >
> > Mark,
> >
> > The update of Mongoose to v6.10 removed the change I had made so that
> > the mg_send() call would transmit on the network immediately if the
> > socket was ready.  I needed to make that change because we would
> > otherwise run out of RAM with SSH because mg_send() would just buffer
> > everything until the next poll.
> >
> >                                                        -- Steve
> >
> > On Fri, 19 Jan 2018, Mark Webb-Johnson wrote:
> >
> >> I re-worked the ovms_server_* framework, and v2 implementation, to use MONGOOSE.
> >>
> >> It seems to be _basically_ working. It can connect/disconnect/etc. Some slight memory saving, but standardising the networking throughout on mongoose should simplify things.
> >>
> >> I am seeing problems with transmitting the FEATURES and PARAMETERS sometimes - particularly in low memory situations. I’m still trying to find out why.
> >>
> >> Regards, Mark.
> >>
> >>> On 17 Jan 2018, at 8:33 AM, Mark Webb-Johnson <mark at webb-johnson.net> wrote:
> >>>
> >>>
> >>> This is the issue Michael pointed out. The 'server response is incomplete’ problem with select(). Apologies for this; I am not sure why I didn’t notice it before.
> >>>
> >>> Gory details are here:
> >>>
> >>> https://github.com/espressif/esp-idf/issues/1510 <https://github.com/espressif/esp-idf/issues/1510>
> >>>
> >>> I think Espressif implemented requirement this in a bizarre way, likely to break compatibility, but such is life. They did point it out as a ‘breaking change’ (at the bottom of the release notes for 3.0b1):
> >>>
> >>> https://github.com/espressif/esp-idf/releases/tag/v3.0-rc1 <https://github.com/espressif/esp-idf/releases/tag/v3.0-rc1>
> >>>
> >>> LWIP socket file descriptors now take higher numeric values (via the LWIP LWIP_SOCKET_OFFSET macro). BSD sockets code should mostly work as expected (and, new in V3.0, some standard POSIX functions can now be used with sockets). However any code which assumes a socket file descriptor is always a low numbered integer may need modifying to account for LWIP_SOCKET_OFFSET.
> >>>
> >>> It sure broke us.
> >>>
> >>> I’ve made a one-line workaround fix (to ovms_buffer.cpp), and ovms server v2 connections are working again for me. That is committed and pushed already.
> >>>
> >>> It is kind of messy to have all these different networking implementations in our code base; I intend to move ovms_server_* to mongoose networking over the next few days. That will mean we won’t need a separate task/stack for server connections, and should save us 7KB internal RAM for each connection. Also ovms_ota. But that will have to wait, as I need to get the hardware complete first (some issues with 1.8v vs 3.3v logic on VDD_SDIO of the wrover module and some of our GPIOs), and that is top priority.
> >>>
> >>> Regards, Mark.
> >>>
> >>>> On 17 Jan 2018, at 7:05 AM, Greg D. <gregd2350 at gmail.com <mailto:gregd2350 at gmail.com>> wrote:
> >>>>
> >>>> But, I'm not getting much love out of the v2 server.  The connection doesn't appear to be working - "server response is incomplete".  Same error whether on wifi or modem.
> >>>
> >>
> > _______________________________________________
> > OvmsDev mailing list
> > OvmsDev at lists.teslaclub.hk
> > http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
>
>