LWIP improvements Topic is solved

[mikeprotts]

I'm now running with LWIP 2.1.2 and ChibiOS 20.3.1:
ST_STM32F4-DISCOVERY with STM32F4DIS-BB (STM32F407)
Olimex STM32-E407_REV_D (STM32F407)
ST_NUCLEO144_F429ZI
ST_NUCLEO144_F746ZG
ST_NUCLEO144_F767ZI
Custom board with STM32F767

Note this includes the changes I suggested for LWIP stats (see earlier post in this thread), and for running with no heap (see other thread viewtopic.php?f=38&t=5531).

I've backported a change from the current master to make LWIP_ERROR not trigger an assert, as it's hitting a problem (mqtt trying to send when link drops generates an error).

I've also included a fix for DAC that's not relevant to this thread, but here for completeness (see bug viewtopic.php?f=35&t=5529).

If you download the ChibiOS_20.3..7z1 stable file, expand ext/lwip-2.1.2.7z and link as ext/lwip, and add my updates you will have the same as me. Files attached for info in case someone comes across this thread some time in the future:
./ChibiOS_20.3.1/ext/lwip/src/core/stats.c
./ChibiOS_20.3.1/ext/lwip/src/include/lwip/debug.h
./ChibiOS_20.3.1/os/hal/ports/STM32/LLD/DACv1/hal_dac_lld.c
./ChibiOS_20.3.1/os/various/lwip_bindings/arch/sys_arch.c

Mike

[mikenick42]

Giovanni,

I can put together a patch with my changes for the iGMP flag. It's only a couple lines, but my concern is that for my quick-and-dirty solution, I added

Code: Select all

#include stlib.h

to lwipopts.h because with the IGMP true I needed to define LWIP_RAND as (uint32)rand() which may or may not be the right way to define it, but pulling stdlib into lwipopts doesn't seem like the right thing to do outside a quick fix.

Thanks,
Mike

[steved]

Actually I think the stats problem is both ChibiiOS restriction & lwip bug.

The code in stats.c:

void
stats_display_mem(struct stats_mem *mem, const char *name)
{
  LWIP_PLATFORM_DIAG(("\nMEM %s\n\t", name));
  LWIP_PLATFORM_DIAG(("avail: %"MEM_SIZE_F"\n\t", mem->avail));
  LWIP_PLATFORM_DIAG(("used: %"MEM_SIZE_F"\n\t", mem->used));
  LWIP_PLATFORM_DIAG(("max: %"MEM_SIZE_F"\n\t", mem->max));
  LWIP_PLATFORM_DIAG(("err: %"STAT_COUNTER_F"\n", mem->err));
}

should be using STAT_COUNTER_F for consistency (I'll report this to LWIP as a bug):

void
stats_display_mem(struct stats_mem *mem, const char *name)
{
  LWIP_PLATFORM_DIAG(("\nMEM %s\n\t", name));
  LWIP_PLATFORM_DIAG(("avail: %"STAT_COUNTER_F"\n\t", mem->avail));
  LWIP_PLATFORM_DIAG(("used: %"STAT_COUNTER_F"\n\t", mem->used));
  LWIP_PLATFORM_DIAG(("max: %"STAT_COUNTER_F"\n\t", mem->max));
  LWIP_PLATFORM_DIAG(("err: %"STAT_COUNTER_F"\n", mem->err));
}

and then if I use:

#define LWIP_STATS_LARGE 1

it works fine. However, with:

#define LWIP_STATS_LARGE 0

all the stats become "hu". As I use the ChibiOS chsnprint function, I checked:
os/hal/lib/streams/chprintf.c

and h is not a supported length modifier (only l and L are). So we can either accept the limitation and require the LWIP_STATS_LARGE 1 setting, or implement a h (short int) modifier. I'll see if it's easy to add support, but I wouldn't consider this a bug as it's as designed in ChibiOS.

Mike

Mike,
Parts of lwIP allows you to override the print format modifiers; in my lwipopts.h I have:

Code: Select all

/* Format characters for various types - chprintf isn't C-compatible
 * Note: The debug strings often don't use the #define values, especially for
 * pointers. Then you get 'p' for pointer, and data is out of step. (Bodge fixed
 * with chprintf() change - subsequently added to Chibi  */
#define U8_F "c"
#define S8_F "c"
#define X8_F "x"
#define U16_F "u"
#define S16_F "d"
#define X16_F "x"
#define U32_F "u"
#define S32_F "d"
#define X32_F "x"
#define SZT_F "08x"        /* Pointer */

[mikeprotts]

Very useful, thanks. My lwipopts.h has just had that added .

Mike

[steved]

Maybe it should be added to the standard Chibi distribution?

[Giovanni]

Sure but define "it" there is a lot of discussion going on about a topic I have no deep understanding of.

Giovanni

[steved]

Sorry, concentrating on my own modest contribution!

I'm referring to the print modifiers in my post of 3rd June; these are necessary if you turn on lwIP's debug features to get correct print formatting. They are Chibi-specific, so it seems logical to package them with Chibi.

[mikeprotts]

I support adding the print modifiers as suggested, I can't see any obvious problems they would cause.

The alternative (making chprintf handle the modifiers) would seem to be more work for little extra benefit.

Mike