LWIP improvements Topic is solved

Use this forum for requesting small changes in ChibiOS. Large changes should be discussed in the development forum. This forum is NOT for support.
mikeprotts
Posts: 151
Joined: Wed Jan 09, 2019 12:37 pm
Has thanked: 19 times
Been thanked: 23 times

Re: LWIP improvements

Postby mikeprotts » Tue Jun 02, 2020 2:00 pm

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
Attachments
ChibiOS_20.3.1.mods.tgz
(9.33 KiB) Downloaded 19 times

mikenick42
Posts: 26
Joined: Tue Mar 10, 2015 4:49 pm
Been thanked: 1 time

Re: LWIP improvements

Postby mikenick42 » Tue Jun 02, 2020 2:19 pm

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
Posts: 734
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 10 times
Been thanked: 109 times

Re: LWIP improvements

Postby steved » Wed Jun 03, 2020 9:06 am

mikeprotts wrote: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
Posts: 151
Joined: Wed Jan 09, 2019 12:37 pm
Has thanked: 19 times
Been thanked: 23 times

Re: LWIP improvements

Postby mikeprotts » Wed Jun 03, 2020 9:42 am

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

Mike

steved
Posts: 734
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 10 times
Been thanked: 109 times

Re: LWIP improvements

Postby steved » Fri Jun 12, 2020 8:58 am

Maybe it should be added to the standard Chibi distribution?

User avatar
Giovanni
Site Admin
Posts: 13133
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 770 times
Been thanked: 653 times
Contact:

Re: LWIP improvements

Postby Giovanni » Fri Jun 12, 2020 10:07 am

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

Giovanni

steved
Posts: 734
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 10 times
Been thanked: 109 times

Re: LWIP improvements

Postby steved » Fri Jun 12, 2020 1:03 pm

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
Posts: 151
Joined: Wed Jan 09, 2019 12:37 pm
Has thanked: 19 times
Been thanked: 23 times

Re: LWIP improvements

Postby mikeprotts » Fri Jun 12, 2020 3:02 pm

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


Return to “Small Change Requests”

Who is online

Users browsing this forum: No registered users and 1 guest