Re: [lwip-users] httpc_get_file doesn't seem to download entire file?

[Bas Prins]

Hi Simon,

I created
lwIP - A Lightweight TCP/IP stack - Bugs: bug #60787, http_client does not
reset timeout... [Savannah] (nongnu.org)


I don't feel confident enough to make the pull request. The suggestions you
did make sense, but I don't know lwip well enough to go further than the
simple one-liner which I changed in http_client.c in my local repo.

Hope the bug report is clear enough.

Best regards, bas

Op wo 16 jun. 2021 om 22:22 schreef Simon Goldschmidt :

>
>
> Am 16. Juni 2021 20:15:02 MESZ schrieb "Rémy DZIEMIASZKO" <
> remy.dziemias...@smile.fr>:
> >Le mer. 16 juin 2021 à 15:24, Bas Prins  a écrit
> >:
> >>
> >> Hi Remy,
> >>
> >> Thanks for your answer.
> >>
> >> It feels a bit awkward this way though. Do you know any reason why
> >user code should be bothered with reloading the timeout ticks? Why
> >doesn't http_client deal with this when it gets notified about a new
> >packet on "httpc_tcp_recv" ?
> >>
> >
> >I agree with you that http_client timeout management shall be improved:
>
> Definitely seems so. Would anyone please file a bug report for this? Or
> even better, a patch fixing this? The timeout should be reset when
> receiving a chunk but should probably be overridable at compile time or
> runtime, too.
>
> Regards,
> Simon
>
> >1) By reloading the timer after each chunk reception (what you have
> >implemented below)
> >2) By giving the possibility for the application to choose the timeout
> >value(s). From simple to more complex possible enhancement:
> >2.a) By turning HTTPC_POLL_TIMEOUT into a lwip option instead of a
> >fixed #define.
> >2.b) By passing an additional parameter to httpc_get_file to configure
> >for each connection a specific timeout value (because timeout value
> >may depend on the server behaviour or network performance).
> >2.c) By passing various parameters for various timeouts: timeout for
> >the initial connection to the server, timeout between chunk
> >receptions, timeout of the complete http request
> >
> >> And no, the opaque type is still as is in the latest and greatest
> >version of lwip. Clearly indicating "I shouldn't be using their
> >httpc_state_t, right?
> >>
> >> Would this make sense to you:
> >>
> >> /** http client tcp recv callback */
> >> static err_t
> >> httpc_tcp_recv(void *arg, struct altcp_pcb *pcb, struct pbuf *p,
> >err_t r)
> >> {
> >>   httpc_state_t* req = (httpc_state_t*)arg;
> >>   LWIP_UNUSED_ARG(r);
> >> ...
> >> ...
> >> ...
> >>   if ((p != NULL) && (req->parse_state == HTTPC_PARSE_RX_DATA)) {
> >> req->rx_content_len += p->tot_len;
> >>
> >>
> >> // - RESET TIMER TICKS HERE --
> >> req->timeout_ticks = HTTPC_POLL_TIMEOUT;
> >>
> >>
> >> if (req->recv_fn != NULL) {
> >>   /* directly return here: the connection migth already be
> >aborted from the callback! */
> >>   return req->recv_fn(req->callback_arg, pcb, p, r);
> >> } else {
> >>   altcp_recved(pcb, p->tot_len);
> >>   pbuf_free(p);
> >> }
> >>   }
> >>   return ERR_OK;
> >> }
> >>
> >> This solves my problem, and this way user code is not bothered with
> >something I think it shouldn't have to bother with. And the
> >httpc_state_t can remain opaque as well.
> >>
> >> Best regards, bas
> >>
> >>
> >>
> >> Op wo 16 jun. 2021 om 14:15 schreef Rémy DZIEMIASZKO
> >:
> >>>
> >>> Hello,
> >>>
> >>> When you call 'httpc_get_file' the parameter 'connection' gives you
> >a
> >>> handle on the http_state used for that connection
> >>> then you can passes this handle to 'httpc_get_file' via the
> >parameter
> >>> 'void * callback_arg'
> >>> then your received function will get the handle via the parameter
> >'void * arg'
> >>> then in the received function you can do (http_state_t
> >>> *)arg->timeout_ticks = MY_RELOAD_VALUE;
> >>>
> >>> http_state_t * foo;
> >>> httpc_get_file(ip, port, uri, settings, my_recv_fn, foo, )
> >>>
> >>> err_t  my_recv_fn(void * arg, ...)
> >>> {
> >>>(http_state_t *)arg->timeout_ticks = MY_RELOAD_VALUE;
> >>> }
> >>>
> >>> You might have a compilation issue because 'http_state_t' is
> >normally
> >>> an opaque type for the application then the member 'timeout_ticks'
> >is
> >>> not visible from the application.
> >>> In a past project I solved that by exporting the definition of
> >>> 'http_state_t' but maybe this is already fixed in the last release
> >of
> >>> lwip.
> >>>
> >>> Regards
> >>> Rémy
> >>>
> >>> Le mer. 16 juin 2021 à 13:24, Bas Prins  a
> >écrit :
> >>> >
> >>> > Dear ,
> >>> >
> >>> > I don't think I am able to reset the timer. The 'arg' passed  in
> >>> >
> >>> > err_t rec_fn(void *arg, struct altcp_pcb *conn, struct pbuf *p,
> >err_t err)
> >>> >
> >>> > is not of type httpc_state_t. The rec_fn callback is called here:
> >>> >
> >>> > static err_t
> >>> > httpc_tcp_recv(void *arg, struct altcp_pcb *pcb, struct pbuf *p,
> >err_t r)
> >>> > ...
> >>> > ...
> >>> > ...
> >>> > if (req->recv_fn != NULL) {
> >>> >   /* directly 

Re: [lwip-users] httpc_get_file doesn't seem to download entire file?

[Simon Goldschmidt]

Am 16. Juni 2021 20:15:02 MESZ schrieb "Rémy DZIEMIASZKO" 
:
>Le mer. 16 juin 2021 à 15:24, Bas Prins  a écrit
>:
>>
>> Hi Remy,
>>
>> Thanks for your answer.
>>
>> It feels a bit awkward this way though. Do you know any reason why
>user code should be bothered with reloading the timeout ticks? Why
>doesn't http_client deal with this when it gets notified about a new
>packet on "httpc_tcp_recv" ?
>>
>
>I agree with you that http_client timeout management shall be improved:

Definitely seems so. Would anyone please file a bug report for this? Or even 
better, a patch fixing this? The timeout should be reset when receiving a chunk 
but should probably be overridable at compile time or runtime, too.

Regards,
Simon

>1) By reloading the timer after each chunk reception (what you have
>implemented below)
>2) By giving the possibility for the application to choose the timeout
>value(s). From simple to more complex possible enhancement:
>2.a) By turning HTTPC_POLL_TIMEOUT into a lwip option instead of a
>fixed #define.
>2.b) By passing an additional parameter to httpc_get_file to configure
>for each connection a specific timeout value (because timeout value
>may depend on the server behaviour or network performance).
>2.c) By passing various parameters for various timeouts: timeout for
>the initial connection to the server, timeout between chunk
>receptions, timeout of the complete http request
>
>> And no, the opaque type is still as is in the latest and greatest
>version of lwip. Clearly indicating "I shouldn't be using their
>httpc_state_t, right?
>>
>> Would this make sense to you:
>>
>> /** http client tcp recv callback */
>> static err_t
>> httpc_tcp_recv(void *arg, struct altcp_pcb *pcb, struct pbuf *p,
>err_t r)
>> {
>>   httpc_state_t* req = (httpc_state_t*)arg;
>>   LWIP_UNUSED_ARG(r);
>> ...
>> ...
>> ...
>>   if ((p != NULL) && (req->parse_state == HTTPC_PARSE_RX_DATA)) {
>> req->rx_content_len += p->tot_len;
>>
>>
>> // - RESET TIMER TICKS HERE --
>> req->timeout_ticks = HTTPC_POLL_TIMEOUT;
>>
>>
>> if (req->recv_fn != NULL) {
>>   /* directly return here: the connection migth already be
>aborted from the callback! */
>>   return req->recv_fn(req->callback_arg, pcb, p, r);
>> } else {
>>   altcp_recved(pcb, p->tot_len);
>>   pbuf_free(p);
>> }
>>   }
>>   return ERR_OK;
>> }
>>
>> This solves my problem, and this way user code is not bothered with
>something I think it shouldn't have to bother with. And the
>httpc_state_t can remain opaque as well.
>>
>> Best regards, bas
>>
>>
>>
>> Op wo 16 jun. 2021 om 14:15 schreef Rémy DZIEMIASZKO
>:
>>>
>>> Hello,
>>>
>>> When you call 'httpc_get_file' the parameter 'connection' gives you
>a
>>> handle on the http_state used for that connection
>>> then you can passes this handle to 'httpc_get_file' via the
>parameter
>>> 'void * callback_arg'
>>> then your received function will get the handle via the parameter
>'void * arg'
>>> then in the received function you can do (http_state_t
>>> *)arg->timeout_ticks = MY_RELOAD_VALUE;
>>>
>>> http_state_t * foo;
>>> httpc_get_file(ip, port, uri, settings, my_recv_fn, foo, )
>>>
>>> err_t  my_recv_fn(void * arg, ...)
>>> {
>>>(http_state_t *)arg->timeout_ticks = MY_RELOAD_VALUE;
>>> }
>>>
>>> You might have a compilation issue because 'http_state_t' is
>normally
>>> an opaque type for the application then the member 'timeout_ticks'
>is
>>> not visible from the application.
>>> In a past project I solved that by exporting the definition of
>>> 'http_state_t' but maybe this is already fixed in the last release
>of
>>> lwip.
>>>
>>> Regards
>>> Rémy
>>>
>>> Le mer. 16 juin 2021 à 13:24, Bas Prins  a
>écrit :
>>> >
>>> > Dear ,
>>> >
>>> > I don't think I am able to reset the timer. The 'arg' passed  in
>>> >
>>> > err_t rec_fn(void *arg, struct altcp_pcb *conn, struct pbuf *p,
>err_t err)
>>> >
>>> > is not of type httpc_state_t. The rec_fn callback is called here:
>>> >
>>> > static err_t
>>> > httpc_tcp_recv(void *arg, struct altcp_pcb *pcb, struct pbuf *p,
>err_t r)
>>> > ...
>>> > ...
>>> > ...
>>> > if (req->recv_fn != NULL) {
>>> >   /* directly return here: the connection migth already be
>aborted from the callback! */
>>> >   return req->recv_fn(req->callback_arg, pcb, p, r);
>>> > }
>>> >
>>> > So what's the deal with this function
>>> >
>>> > /** http client tcp poll callback */
>>> > static err_t
>>> > httpc_tcp_poll(void *arg, struct altcp_pcb *pcb)
>>> > {
>>> >   /* implement timeout */
>>> >   httpc_state_t* req = (httpc_state_t*)arg;
>>> >   LWIP_UNUSED_ARG(pcb);
>>> >   if (req != NULL) {
>>> > if (req->timeout_ticks) {
>>> >   req->timeout_ticks--;
>>> > }
>>> > if (!req->timeout_ticks) {
>>> >   return httpc_close(req, HTTPC_RESULT_ERR_TIMEOUT, 0,
>ERR_OK);
>>> > }
>>> >   }
>>> >   return ERR_OK;
>>> > }
>>> >
>>> > It only decreases the ticks counter. It never gets reset. So I am
>not allowed to download files 

Re: [lwip-users] httpc_get_file doesn't seem to download entire file?

[Rémy DZIEMIASZKO]

Le mer. 16 juin 2021 à 15:24, Bas Prins  a écrit :
>
> Hi Remy,
>
> Thanks for your answer.
>
> It feels a bit awkward this way though. Do you know any reason why user code 
> should be bothered with reloading the timeout ticks? Why doesn't http_client 
> deal with this when it gets notified about a new packet on "httpc_tcp_recv" ?
>

I agree with you that http_client timeout management shall be improved:
1) By reloading the timer after each chunk reception (what you have
implemented below)
2) By giving the possibility for the application to choose the timeout
value(s). From simple to more complex possible enhancement:
2.a) By turning HTTPC_POLL_TIMEOUT into a lwip option instead of a
fixed #define.
2.b) By passing an additional parameter to httpc_get_file to configure
for each connection a specific timeout value (because timeout value
may depend on the server behaviour or network performance).
2.c) By passing various parameters for various timeouts: timeout for
the initial connection to the server, timeout between chunk
receptions, timeout of the complete http request

> And no, the opaque type is still as is in the latest and greatest version of 
> lwip. Clearly indicating "I shouldn't be using their httpc_state_t, right?
>
> Would this make sense to you:
>
> /** http client tcp recv callback */
> static err_t
> httpc_tcp_recv(void *arg, struct altcp_pcb *pcb, struct pbuf *p, err_t r)
> {
>   httpc_state_t* req = (httpc_state_t*)arg;
>   LWIP_UNUSED_ARG(r);
> ...
> ...
> ...
>   if ((p != NULL) && (req->parse_state == HTTPC_PARSE_RX_DATA)) {
> req->rx_content_len += p->tot_len;
>
>
> // - RESET TIMER TICKS HERE --
> req->timeout_ticks = HTTPC_POLL_TIMEOUT;
>
>
> if (req->recv_fn != NULL) {
>   /* directly return here: the connection migth already be aborted from 
> the callback! */
>   return req->recv_fn(req->callback_arg, pcb, p, r);
> } else {
>   altcp_recved(pcb, p->tot_len);
>   pbuf_free(p);
> }
>   }
>   return ERR_OK;
> }
>
> This solves my problem, and this way user code is not bothered with something 
> I think it shouldn't have to bother with. And the httpc_state_t can remain 
> opaque as well.
>
> Best regards, bas
>
>
>
> Op wo 16 jun. 2021 om 14:15 schreef Rémy DZIEMIASZKO 
> :
>>
>> Hello,
>>
>> When you call 'httpc_get_file' the parameter 'connection' gives you a
>> handle on the http_state used for that connection
>> then you can passes this handle to 'httpc_get_file' via the parameter
>> 'void * callback_arg'
>> then your received function will get the handle via the parameter 'void * 
>> arg'
>> then in the received function you can do (http_state_t
>> *)arg->timeout_ticks = MY_RELOAD_VALUE;
>>
>> http_state_t * foo;
>> httpc_get_file(ip, port, uri, settings, my_recv_fn, foo, )
>>
>> err_t  my_recv_fn(void * arg, ...)
>> {
>>(http_state_t *)arg->timeout_ticks = MY_RELOAD_VALUE;
>> }
>>
>> You might have a compilation issue because 'http_state_t' is normally
>> an opaque type for the application then the member 'timeout_ticks' is
>> not visible from the application.
>> In a past project I solved that by exporting the definition of
>> 'http_state_t' but maybe this is already fixed in the last release of
>> lwip.
>>
>> Regards
>> Rémy
>>
>> Le mer. 16 juin 2021 à 13:24, Bas Prins  a écrit :
>> >
>> > Dear ,
>> >
>> > I don't think I am able to reset the timer. The 'arg' passed  in
>> >
>> > err_t rec_fn(void *arg, struct altcp_pcb *conn, struct pbuf *p, err_t err)
>> >
>> > is not of type httpc_state_t. The rec_fn callback is called here:
>> >
>> > static err_t
>> > httpc_tcp_recv(void *arg, struct altcp_pcb *pcb, struct pbuf *p, err_t r)
>> > ...
>> > ...
>> > ...
>> > if (req->recv_fn != NULL) {
>> >   /* directly return here: the connection migth already be aborted 
>> > from the callback! */
>> >   return req->recv_fn(req->callback_arg, pcb, p, r);
>> > }
>> >
>> > So what's the deal with this function
>> >
>> > /** http client tcp poll callback */
>> > static err_t
>> > httpc_tcp_poll(void *arg, struct altcp_pcb *pcb)
>> > {
>> >   /* implement timeout */
>> >   httpc_state_t* req = (httpc_state_t*)arg;
>> >   LWIP_UNUSED_ARG(pcb);
>> >   if (req != NULL) {
>> > if (req->timeout_ticks) {
>> >   req->timeout_ticks--;
>> > }
>> > if (!req->timeout_ticks) {
>> >   return httpc_close(req, HTTPC_RESULT_ERR_TIMEOUT, 0, ERR_OK);
>> > }
>> >   }
>> >   return ERR_OK;
>> > }
>> >
>> > It only decreases the ticks counter. It never gets reset. So I am not 
>> > allowed to download files longer than 30 secs? If the download doesn't 
>> > succeed within that time, http_client closes the connection.
>> >
>> > I would expect that either the timeout counter is reset every chunk of 
>> > data which is received, or that user code (my code) would receive a 
>> > pointer to the httpc_state_t so that it could reset the timer.
>> >
>> > Can somebody please explain where I am wrong, what am I missing, or that 
>> > 

Re: [lwip-users] httpc_get_file doesn't seem to download entire file?

[Bas Prins]

Hi Remy,

Thanks for your answer.

It feels a bit awkward this way though. Do you know any reason why user
code should be bothered with reloading the timeout ticks? Why doesn't
http_client deal with this when it gets notified about a new packet on "
httpc_tcp_recv" ?

And no, the opaque type is still as is in the latest and greatest version
of lwip. Clearly indicating "I shouldn't be using their httpc_state_t,
right?

Would this make sense to you:

/** http client tcp recv callback */
static err_t
httpc_tcp_recv(void *arg, struct altcp_pcb *pcb, struct pbuf *p, err_t r)
{
  httpc_state_t* req = (httpc_state_t*)arg;
  LWIP_UNUSED_ARG(r);
...
...
...
  if ((p != NULL) && (req->parse_state == HTTPC_PARSE_RX_DATA)) {
req->rx_content_len += p->tot_len;


// - RESET TIMER TICKS HERE --
req->timeout_ticks = HTTPC_POLL_TIMEOUT;


if (req->recv_fn != NULL) {

/* directly return here: the connection migth already be aborted from
the callback! */
  return req->recv_fn(req->callback_arg, pcb, p, r);
} else {
  altcp_recved(pcb, p->tot_len);
  pbuf_free(p);
}
  }
  return ERR_OK;
}

This solves my problem, and this way user code is not bothered with
something I think it shouldn't have to bother with. And the httpc_state_t
can remain opaque as well.

Best regards, bas



Op wo 16 jun. 2021 om 14:15 schreef Rémy DZIEMIASZKO <
remy.dziemias...@smile.fr>:

> Hello,
>
> When you call 'httpc_get_file' the parameter 'connection' gives you a
> handle on the http_state used for that connection
> then you can passes this handle to 'httpc_get_file' via the parameter
> 'void * callback_arg'
> then your received function will get the handle via the parameter 'void *
> arg'
> then in the received function you can do (http_state_t
> *)arg->timeout_ticks = MY_RELOAD_VALUE;
>
> http_state_t * foo;
> httpc_get_file(ip, port, uri, settings, my_recv_fn, foo, )
>
> err_t  my_recv_fn(void * arg, ...)
> {
>(http_state_t *)arg->timeout_ticks = MY_RELOAD_VALUE;
> }
>
> You might have a compilation issue because 'http_state_t' is normally
> an opaque type for the application then the member 'timeout_ticks' is
> not visible from the application.
> In a past project I solved that by exporting the definition of
> 'http_state_t' but maybe this is already fixed in the last release of
> lwip.
>
> Regards
> Rémy
>
> Le mer. 16 juin 2021 à 13:24, Bas Prins  a écrit :
> >
> > Dear ,
> >
> > I don't think I am able to reset the timer. The 'arg' passed  in
> >
> > err_t rec_fn(void *arg, struct altcp_pcb *conn, struct pbuf *p, err_t
> err)
> >
> > is not of type httpc_state_t. The rec_fn callback is called here:
> >
> > static err_t
> > httpc_tcp_recv(void *arg, struct altcp_pcb *pcb, struct pbuf *p, err_t r)
> > ...
> > ...
> > ...
> > if (req->recv_fn != NULL) {
> >   /* directly return here: the connection migth already be aborted
> from the callback! */
> >   return req->recv_fn(req->callback_arg, pcb, p, r);
> > }
> >
> > So what's the deal with this function
> >
> > /** http client tcp poll callback */
> > static err_t
> > httpc_tcp_poll(void *arg, struct altcp_pcb *pcb)
> > {
> >   /* implement timeout */
> >   httpc_state_t* req = (httpc_state_t*)arg;
> >   LWIP_UNUSED_ARG(pcb);
> >   if (req != NULL) {
> > if (req->timeout_ticks) {
> >   req->timeout_ticks--;
> > }
> > if (!req->timeout_ticks) {
> >   return httpc_close(req, HTTPC_RESULT_ERR_TIMEOUT, 0, ERR_OK);
> > }
> >   }
> >   return ERR_OK;
> > }
> >
> > It only decreases the ticks counter. It never gets reset. So I am not
> allowed to download files longer than 30 secs? If the download doesn't
> succeed within that time, http_client closes the connection.
> >
> > I would expect that either the timeout counter is reset every chunk of
> data which is received, or that user code (my code) would receive a pointer
> to the httpc_state_t so that it could reset the timer.
> >
> > Can somebody please explain where I am wrong, what am I missing, or that
> indeed the current implementation of http_client is lacking ?
> >
> > Many thanks in advance
> >
> > best regards, bas
> >
> >
> >
> > Op wo 16 jun. 2021 om 09:29 schreef Bas Prins :
> >>
> >> Hi all,
> >>
> >> I found the problem, which was rather easy to find once I convinced
> myself I am man enough to actually read code ;-).
> >>
> >> But now I wonder how to solve this appropriately. The problem seems to
> be, that this function decreases the ticks, until the timeout period is
> consumed, and then simply closes the connection.
> >>
> >> /** http client tcp poll callback */
> >> static err_t
> >> httpc_tcp_poll(void *arg, struct altcp_pcb *pcb)
> >> {
> >>   /* implement timeout */
> >>   httpc_state_t* req = (httpc_state_t*)arg;
> >>   LWIP_UNUSED_ARG(pcb);
> >>   if (req != NULL) {
> >> if (req->timeout_ticks) {
> >>   req->timeout_ticks--;
> >> }
> >> if (!req->timeout_ticks) {
> >>   return httpc_close(req, HTTPC_RESULT_ERR_TIMEOUT, 0, 

Re: [lwip-users] httpc_get_file doesn't seem to download entire file?

[Rémy DZIEMIASZKO]

Hello,

When you call 'httpc_get_file' the parameter 'connection' gives you a
handle on the http_state used for that connection
then you can passes this handle to 'httpc_get_file' via the parameter
'void * callback_arg'
then your received function will get the handle via the parameter 'void * arg'
then in the received function you can do (http_state_t
*)arg->timeout_ticks = MY_RELOAD_VALUE;

http_state_t * foo;
httpc_get_file(ip, port, uri, settings, my_recv_fn, foo, )

err_t  my_recv_fn(void * arg, ...)
{
   (http_state_t *)arg->timeout_ticks = MY_RELOAD_VALUE;
}

You might have a compilation issue because 'http_state_t' is normally
an opaque type for the application then the member 'timeout_ticks' is
not visible from the application.
In a past project I solved that by exporting the definition of
'http_state_t' but maybe this is already fixed in the last release of
lwip.

Regards
Rémy

Le mer. 16 juin 2021 à 13:24, Bas Prins  a écrit :
>
> Dear ,
>
> I don't think I am able to reset the timer. The 'arg' passed  in
>
> err_t rec_fn(void *arg, struct altcp_pcb *conn, struct pbuf *p, err_t err)
>
> is not of type httpc_state_t. The rec_fn callback is called here:
>
> static err_t
> httpc_tcp_recv(void *arg, struct altcp_pcb *pcb, struct pbuf *p, err_t r)
> ...
> ...
> ...
> if (req->recv_fn != NULL) {
>   /* directly return here: the connection migth already be aborted from 
> the callback! */
>   return req->recv_fn(req->callback_arg, pcb, p, r);
> }
>
> So what's the deal with this function
>
> /** http client tcp poll callback */
> static err_t
> httpc_tcp_poll(void *arg, struct altcp_pcb *pcb)
> {
>   /* implement timeout */
>   httpc_state_t* req = (httpc_state_t*)arg;
>   LWIP_UNUSED_ARG(pcb);
>   if (req != NULL) {
> if (req->timeout_ticks) {
>   req->timeout_ticks--;
> }
> if (!req->timeout_ticks) {
>   return httpc_close(req, HTTPC_RESULT_ERR_TIMEOUT, 0, ERR_OK);
> }
>   }
>   return ERR_OK;
> }
>
> It only decreases the ticks counter. It never gets reset. So I am not allowed 
> to download files longer than 30 secs? If the download doesn't succeed within 
> that time, http_client closes the connection.
>
> I would expect that either the timeout counter is reset every chunk of data 
> which is received, or that user code (my code) would receive a pointer to the 
> httpc_state_t so that it could reset the timer.
>
> Can somebody please explain where I am wrong, what am I missing, or that 
> indeed the current implementation of http_client is lacking ?
>
> Many thanks in advance
>
> best regards, bas
>
>
>
> Op wo 16 jun. 2021 om 09:29 schreef Bas Prins :
>>
>> Hi all,
>>
>> I found the problem, which was rather easy to find once I convinced myself I 
>> am man enough to actually read code ;-).
>>
>> But now I wonder how to solve this appropriately. The problem seems to be, 
>> that this function decreases the ticks, until the timeout period is 
>> consumed, and then simply closes the connection.
>>
>> /** http client tcp poll callback */
>> static err_t
>> httpc_tcp_poll(void *arg, struct altcp_pcb *pcb)
>> {
>>   /* implement timeout */
>>   httpc_state_t* req = (httpc_state_t*)arg;
>>   LWIP_UNUSED_ARG(pcb);
>>   if (req != NULL) {
>> if (req->timeout_ticks) {
>>   req->timeout_ticks--;
>> }
>> if (!req->timeout_ticks) {
>>   return httpc_close(req, HTTPC_RESULT_ERR_TIMEOUT, 0, ERR_OK);
>> }
>>   }
>>   return ERR_OK;
>> }
>>
>> So I wonder, who should be responsible for resetting the timeout counter? My 
>> code? The definition of the timeout_ticks seems to only visible for to 
>> http_client.c.
>>
>> If I simply comment the line which decreases the counter the download works.
>>
>> Am I still missing the obvious here?
>>
>> Many thanks.
>>
>> best regards, bas
>>
>> Op ma 14 jun. 2021 om 22:05 schreef Bas Prins :
>>>
>>> Hi Simon, all,
>>>
>>> Fixed the problems I had which led to missing a byte occasionally.
>>>
>>> Still unable to download a file using http_client.h. I did learn a small 
>>> part. I log all arguments of
>>>
>>> void result_fn(void *arg, httpc_result_t httpc_result, u32_t 
>>> rx_content_len, u32_t srv_res, err_t err)
>>>
>>> Which produces this log line:
>>> 2021-06-14 21:31:52,041 [DEBUG][thread: 5][UartReader] DOWNLOAD finished: 
>>> httpc_result=5, rx_content_len=5019, srv_res=0, error=0
>>>
>>>   /** Connection timed out (server didn't respond in time) */
>>>   HTTPC_RESULT_ERR_TIMEOUT   = 5,
>>>
>>> I attached the pcap file of tcpdump.
>>> - It contains a couple of attempts to download the file (513KB.zip).
>>> - It shows TCP window full warnings. I don't care too much about that now. 
>>> I'll deal with that once I have a stable download going.
>>> - It shows the result of the timeout where my STM board closes the 
>>> connection.
>>>
>>> I also attached my lwipopts.h and the loggings produced by my app 
>>> (including lwip ppp logging).
>>>
>>> Can you help me understand what is going wrong?
>>> Why is LWIP 

Re: [lwip-users] httpc_get_file doesn't seem to download entire file?

[Bas Prins]

Dear ,

I don't think I am able to reset the timer. The 'arg' passed  in

err_t rec_fn(void *arg, struct altcp_pcb *conn, struct pbuf *p, err_t err)

is not of type httpc_state_t. The rec_fn callback is called here:

static err_t
httpc_tcp_recv(void *arg, struct altcp_pcb *pcb, struct pbuf *p, err_t r)
...
...
...
if (req->recv_fn != NULL) {

/* directly return here: the connection migth already be aborted from
the callback! */
  return req->recv_fn(req->callback_arg, pcb, p, r);
}

So what's the deal with this function

/** http client tcp poll callback */
static err_t
httpc_tcp_poll(void *arg, struct altcp_pcb *pcb)
{
  /* implement timeout */
  httpc_state_t* req = (httpc_state_t*)arg;
  LWIP_UNUSED_ARG(pcb);
  if (req != NULL) {
if (req->timeout_ticks) {
  req->timeout_ticks--;
}
if (!req->timeout_ticks) {
  return httpc_close(req, HTTPC_RESULT_ERR_TIMEOUT, 0, ERR_OK);
}
  }
  return ERR_OK;
}

It only decreases the ticks counter. It never gets reset. So I am not
allowed to download files longer than 30 secs? If the download doesn't
succeed within that time, http_client closes the connection.

I would expect that either the timeout counter is reset every chunk of data
which is received, or that user code (my code) would receive a pointer to
the httpc_state_t so that it could reset the timer.

Can somebody please explain where I am wrong, what am I missing, or that
indeed the current implementation of http_client is lacking ?

Many thanks in advance

best regards, bas



Op wo 16 jun. 2021 om 09:29 schreef Bas Prins :

> Hi all,
>
> I found the problem, which was rather easy to find once I convinced myself
> I am man enough to actually read code ;-).
>
> But now I wonder how to solve this appropriately. The problem seems to be,
> that this function decreases the ticks, until the timeout period is
> consumed, and then simply closes the connection.
>
> /** http client tcp poll callback */
> static err_t
> httpc_tcp_poll(void *arg, struct altcp_pcb *pcb)
> {
>   /* implement timeout */
>   httpc_state_t* req = (httpc_state_t*)arg;
>   LWIP_UNUSED_ARG(pcb);
>   if (req != NULL) {
> if (req->timeout_ticks) {
>   req->timeout_ticks--;
> }
> if (!req->timeout_ticks) {
>   return httpc_close(req, HTTPC_RESULT_ERR_TIMEOUT, 0, ERR_OK);
> }
>   }
>   return ERR_OK;
> }
>
> So I wonder, who should be responsible for resetting the timeout counter?
> My code? The definition of the timeout_ticks seems to only visible for to
> http_client.c.
>
> If I simply comment the line which decreases the counter the download
> works.
>
> Am I still missing the obvious here?
>
> Many thanks.
>
> best regards, bas
>
> Op ma 14 jun. 2021 om 22:05 schreef Bas Prins :
>
>> Hi Simon, all,
>>
>> Fixed the problems I had which led to missing a byte occasionally.
>>
>> Still unable to download a file using http_client.h. I did learn a small
>> part. I log all arguments of
>>
>> void result_fn(void *arg, httpc_result_t httpc_result, u32_t
>> rx_content_len, u32_t srv_res, err_t err)
>>
>> Which produces this log line:
>> 2021-06-14 21:31:52,041 [DEBUG][thread: 5][UartReader] DOWNLOAD finished:*
>> httpc_result=5*, rx_content_len=5019, srv_res=0, error=0
>>
>>   /** Connection timed out (server didn't respond in time) */
>>   HTTPC_RESULT_ERR_TIMEOUT   = 5,
>>
>> I attached the pcap file of tcpdump.
>> - It contains a couple of attempts to download the file (513KB.zip).
>> - It shows TCP window full warnings. I don't care too much about that
>> now. I'll deal with that once I have a stable download going.
>> - It shows the result of the timeout where my STM board closes the
>> connection.
>>
>> I also attached my lwipopts.h and the loggings produced by my app
>> (including lwip ppp logging).
>>
>> Can you help me understand what is going wrong?
>> Why is LWIP complaining about the server not responding in time, while it
>> seems that the server is doing just fine?
>>
>> PS: obviously I tested the download on my desktop, I am able to download
>> the file repeatedly.
>>
>> Many thanks in advance!
>>
>> Best regards, Bas
>>
>>
>> Op vr 11 jun. 2021 om 07:15 schreef goldsi...@gmx.de :
>>
>>> Am 10.06.2021 um 17:27 schrieb Bas Prins:
>>> > Hi Simon,
>>> >
>>> > Thanks, that's a very likely explanation already. I just added the
>>> > pbuf_free(p) but run into an assert occasionally.
>>>
>>> You might have to check for 'p != NULL' just like in tcp recv callback
>>> functions.
>>>
>>> Regards,
>>> Simon
>>>
>>> ___
>>> lwip-users mailing list
>>> lwip-users@nongnu.org
>>> https://lists.nongnu.org/mailman/listinfo/lwip-users
>>
>>
___
lwip-users mailing list
lwip-users@nongnu.org
https://lists.nongnu.org/mailman/listinfo/lwip-users

Re: [lwip-users] httpc_get_file doesn't seem to download entire file?

[Bas Prins]

Hi all,

I found the problem, which was rather easy to find once I convinced myself
I am man enough to actually read code ;-).

But now I wonder how to solve this appropriately. The problem seems to be,
that this function decreases the ticks, until the timeout period is
consumed, and then simply closes the connection.

/** http client tcp poll callback */
static err_t
httpc_tcp_poll(void *arg, struct altcp_pcb *pcb)
{
  /* implement timeout */
  httpc_state_t* req = (httpc_state_t*)arg;
  LWIP_UNUSED_ARG(pcb);
  if (req != NULL) {
if (req->timeout_ticks) {
  req->timeout_ticks--;
}
if (!req->timeout_ticks) {
  return httpc_close(req, HTTPC_RESULT_ERR_TIMEOUT, 0, ERR_OK);
}
  }
  return ERR_OK;
}

So I wonder, who should be responsible for resetting the timeout counter?
My code? The definition of the timeout_ticks seems to only visible for to
http_client.c.

If I simply comment the line which decreases the counter the download
works.

Am I still missing the obvious here?

Many thanks.

best regards, bas

Op ma 14 jun. 2021 om 22:05 schreef Bas Prins :

> Hi Simon, all,
>
> Fixed the problems I had which led to missing a byte occasionally.
>
> Still unable to download a file using http_client.h. I did learn a small
> part. I log all arguments of
>
> void result_fn(void *arg, httpc_result_t httpc_result, u32_t
> rx_content_len, u32_t srv_res, err_t err)
>
> Which produces this log line:
> 2021-06-14 21:31:52,041 [DEBUG][thread: 5][UartReader] DOWNLOAD finished:*
> httpc_result=5*, rx_content_len=5019, srv_res=0, error=0
>
>   /** Connection timed out (server didn't respond in time) */
>   HTTPC_RESULT_ERR_TIMEOUT   = 5,
>
> I attached the pcap file of tcpdump.
> - It contains a couple of attempts to download the file (513KB.zip).
> - It shows TCP window full warnings. I don't care too much about that now.
> I'll deal with that once I have a stable download going.
> - It shows the result of the timeout where my STM board closes the
> connection.
>
> I also attached my lwipopts.h and the loggings produced by my app
> (including lwip ppp logging).
>
> Can you help me understand what is going wrong?
> Why is LWIP complaining about the server not responding in time, while it
> seems that the server is doing just fine?
>
> PS: obviously I tested the download on my desktop, I am able to download
> the file repeatedly.
>
> Many thanks in advance!
>
> Best regards, Bas
>
>
> Op vr 11 jun. 2021 om 07:15 schreef goldsi...@gmx.de :
>
>> Am 10.06.2021 um 17:27 schrieb Bas Prins:
>> > Hi Simon,
>> >
>> > Thanks, that's a very likely explanation already. I just added the
>> > pbuf_free(p) but run into an assert occasionally.
>>
>> You might have to check for 'p != NULL' just like in tcp recv callback
>> functions.
>>
>> Regards,
>> Simon
>>
>> ___
>> lwip-users mailing list
>> lwip-users@nongnu.org
>> https://lists.nongnu.org/mailman/listinfo/lwip-users
>
>
___
lwip-users mailing list
lwip-users@nongnu.org
https://lists.nongnu.org/mailman/listinfo/lwip-users

Re: [lwip-users] httpc_get_file doesn't seem to download entire file?

[goldsi...@gmx.de]

Am 10.06.2021 um 17:27 schrieb Bas Prins:
> Hi Simon,
>
> Thanks, that's a very likely explanation already. I just added the
> pbuf_free(p) but run into an assert occasionally.

You might have to check for 'p != NULL' just like in tcp recv callback
functions.

Regards,
Simon

___
lwip-users mailing list
lwip-users@nongnu.org
https://lists.nongnu.org/mailman/listinfo/lwip-users

Re: [lwip-users] httpc_get_file doesn't seem to download entire file?

[Bas Prins]

Hi Simon,

Thanks, that's a very likely explanation already. I just added the
pbuf_free(p) but run into an assert occasionally.

The pbuf is already freed, but that happens always after a bad fcs. The bad
fcs is because I miss a byte from the received ppp packet.

So I will fix that problem first, and then will see where I end up.

If it still fails to download I will share the pcap files and hope you can
have a look.

Thanks

Best regards Bas



Op wo 9 jun. 2021 om 13:42 schreef goldsi...@gmx.de :

> Am 09.06.2021 um 12:42 schrieb Bas Prins:
> > Hi
> >
> > I am having a hard time understanding the http_client.h interface.
> >
> > My setup:
> > - freertos latest version
> > - lwip 2.1.2 (over pppos)
> > - the only tasks running are lwip related (tcpip thread (task prio=2 +
> > rx thread (task prio=2).
> > - both are over dimensioned for stack and heap (because I'm still
> > developing, I'll reduce to decent amounts when I need to)
> > - uart2 rx/tx are connected to LTE modem. Both rx and tx is used in
> > interrupt mode.
> > - uart3 tx is used for logging to PC (where some simple C# app reads all
> > data and logs to file)
> >
> > I am connecting to a web-server where I want to download a "512.KB.zip"
> > (zip contains \0 characters, I took it from some download speed test
> > server).
> >
> > I experimented with lwftp , but it
>
> I don't know that one.
>
> > seems to leak memory the way I use it. So I thought, why not just
> > download files using http get and use the lwip provided http_client.
> >
> > The following code is used to initiate the download :
> >
> > LogLineToUart("connecting to server...\n");
> >
> > ip_addr_t ip;
> > uint16_t port= 80;
> > httpc_connection_t settings;
> >
> > IP4_ADDR(, 31,21,236,6);
> > settings.result_fn = result_fn;
> >
> > err_t err = httpc_get_file(
> > ,
> > port,
> > "/512KB.zip",
> > ,
> > rec_fn,
> > nullptr,
> > nullptr);
> >
> > if ( err != ERR_OK )
> > {
> > LogLineToUart("httpc_get_file failed (%d)", err);
> > }
> >
> > for now, I only log when I am receiving data:
> >
> >
> err_t rec_fn(void *arg, struct altcp_pcb *conn, struct pbuf *p, err_t err)
> > {
> > LogLineToUart("sinking: %d\n", p->len);
>
> You should be freeing the pbuf here or you'll run out of pbufs eventually.
>
> > }
> >
> >
> void result_fn(void *arg, httpc_result_t httpc_result, u32_t rx_content_len, 
> u32_t srv_res, err_t err)
> > {
> > LogLineToUart("download finished\n");
> > }
> >
> > Both functions are called. So data is being received.
> >
> > The questions arise when I look at wireshark.
> >
> > 1278302.433400192.168.1.113185.99.25.76TCP5880 → 62510 [SYN, ACK] Seq=0
> > Ack=1 Win=29200 Len=0 MSS=1460
> > 1282302.871908185.99.25.76192.168.1.113HTTP205GET /ftp_100kb_5.log
> HTTP/1.1
>
> This is not requesting "/512KB.zip" as in the code above?
>
> > 1283302.872167192.168.1.113185.99.25.76TCP5480 → 62510 [ACK] Seq=1
> > Ack=152 Win=29736 Len=0
> > 1284302.873849192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=1
> > Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> > 1285302.874100192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=473
> > Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> > 1286302.874253192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=945
> > Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> > 1287302.874394192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=1417
> > Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> > 1288302.874535192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=1889
> > Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> > 1289302.874688192.168.1.113185.99.25.76TCP52680 → 62510 [PSH, ACK]
> > Seq=2361 Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> > 1290302.874835192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=2833
> > Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> > 1291302.874975192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=3305
> > Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> > 1292302.875118192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=3777
> > Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> > 1293302.875261192.168.1.113185.99.25.76TCP526[TCP Window Full] 80 →
> > 62510 [ACK] Seq=4249 Ack=152 Win=29736 Len=472 [TCP segment of a
> > reassembled PDU]
> > 1295303.465207185.99.25.76192.168.1.113TCP6062510 → 80 [ACK] Seq=152
> > Ack=473 Win=4248 Len=0
> > 1304304.795607192.168.1.113185.99.25.76TCP526[TCP Retransmission] 80 →
> > 62510 [ACK] Seq=473 Ack=152 Win=29736 Len=472
> > 1305305.354254185.99.25.76192.168.1.113TCP60[TCP Dup ACK 1295#1] 62510 →
> > 80 [ACK] Seq=152 Ack=473 Win=4248 Len=0
> > 1311307.485583192.168.1.113185.99.25.76TCP526[TCP Retransmission] 80 →
> > 62510 [ACK] Seq=473 Ack=152 Win=29736 Len=472
> > 

Re: [lwip-users] httpc_get_file doesn't seem to download entire file?

[goldsi...@gmx.de]

Am 09.06.2021 um 12:42 schrieb Bas Prins:
> Hi
> 
> I am having a hard time understanding the http_client.h interface. 
> 
> My setup:
> - freertos latest version
> - lwip 2.1.2 (over pppos)
> - the only tasks running are lwip related (tcpip thread (task prio=2 +
> rx thread (task prio=2).
> - both are over dimensioned for stack and heap (because I'm still
> developing, I'll reduce to decent amounts when I need to)
> - uart2 rx/tx are connected to LTE modem. Both rx and tx is used in
> interrupt mode.
> - uart3 tx is used for logging to PC (where some simple C# app reads all
> data and logs to file)
> 
> I am connecting to a web-server where I want to download a "512.KB.zip"
> (zip contains \0 characters, I took it from some download speed test
> server).
> 
> I experimented with lwftp , but it

I don't know that one.

> seems to leak memory the way I use it. So I thought, why not just
> download files using http get and use the lwip provided http_client.
> 
> The following code is used to initiate the download :
> 
> LogLineToUart("connecting to server...\n");
> 
> ip_addr_t ip;
> uint16_t port= 80;
> httpc_connection_t settings;
> 
> IP4_ADDR(, 31,21,236,6);
> settings.result_fn = result_fn;
> 
> err_t err = httpc_get_file(
> , 
> port, 
> "/512KB.zip", 
> , 
> rec_fn, 
> nullptr, 
> nullptr);
> 
> if ( err != ERR_OK ) 
> {
> LogLineToUart("httpc_get_file failed (%d)", err);
> }
> 
> for now, I only log when I am receiving data:
> 
> err_t rec_fn(void *arg, struct altcp_pcb *conn, struct pbuf *p, err_t err)
> {
> LogLineToUart("sinking: %d\n", p->len);

You should be freeing the pbuf here or you'll run out of pbufs eventually.

> }
> 
> void result_fn(void *arg, httpc_result_t httpc_result, u32_t rx_content_len, 
> u32_t srv_res, err_t err)
> {
> LogLineToUart("download finished\n");
> }
> 
> Both functions are called. So data is being received. 
> 
> The questions arise when I look at wireshark. 
> 
> 1278302.433400192.168.1.113185.99.25.76TCP5880 → 62510 [SYN, ACK] Seq=0
> Ack=1 Win=29200 Len=0 MSS=1460
> 1282302.871908185.99.25.76192.168.1.113HTTP205GET /ftp_100kb_5.log HTTP/1.1 

This is not requesting "/512KB.zip" as in the code above?

> 1283302.872167192.168.1.113185.99.25.76TCP5480 → 62510 [ACK] Seq=1
> Ack=152 Win=29736 Len=0
> 1284302.873849192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=1
> Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> 1285302.874100192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=473
> Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> 1286302.874253192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=945
> Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> 1287302.874394192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=1417
> Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> 1288302.874535192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=1889
> Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> 1289302.874688192.168.1.113185.99.25.76TCP52680 → 62510 [PSH, ACK]
> Seq=2361 Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> 1290302.874835192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=2833
> Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> 1291302.874975192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=3305
> Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> 1292302.875118192.168.1.113185.99.25.76TCP52680 → 62510 [ACK] Seq=3777
> Ack=152 Win=29736 Len=472 [TCP segment of a reassembled PDU]
> 1293302.875261192.168.1.113185.99.25.76TCP526[TCP Window Full] 80 →
> 62510 [ACK] Seq=4249 Ack=152 Win=29736 Len=472 [TCP segment of a
> reassembled PDU]
> 1295303.465207185.99.25.76192.168.1.113TCP6062510 → 80 [ACK] Seq=152
> Ack=473 Win=4248 Len=0
> 1304304.795607192.168.1.113185.99.25.76TCP526[TCP Retransmission] 80 →
> 62510 [ACK] Seq=473 Ack=152 Win=29736 Len=472
> 1305305.354254185.99.25.76192.168.1.113TCP60[TCP Dup ACK 1295#1] 62510 →
> 80 [ACK] Seq=152 Ack=473 Win=4248 Len=0
> 1311307.485583192.168.1.113185.99.25.76TCP526[TCP Retransmission] 80 →
> 62510 [ACK] Seq=473 Ack=152 Win=29736 Len=472
> 1312307.903785185.99.25.76192.168.1.113TCP60[TCP Dup ACK 1295#2] 62510 →
> 80 [ACK] Seq=152 Ack=473 Win=4248 Len=0
> 1324312.855604192.168.1.113185.99.25.76TCP526[TCP Retransmission] 80 →
> 62510 [ACK] Seq=473 Ack=152 Win=29736 Len=472
> 1326313.354377185.99.25.76192.168.1.113TCP60[TCP Dup ACK 1295#3] 62510 →
> 80 [ACK] Seq=152 Ack=473 Win=4248 Len=0
> 1334317.187014185.99.25.76192.168.1.113TCP6062510 → 80 [RST, ACK]
> Seq=152 Ack=473 Win=28690 Len=0
> 
> The data sent by the server doesn't add up to 512KB by a long shot. And
> my STM doesn't seem to send acks fast enough, resulting in window full +
> retransmits.

Please share pcap files, not text summaries. You can see in the pcap
which data is being sent by the server. That