-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create sDDF LWIP Library #238
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General idea is good, some minor issues.
examples/echo_server/echo.mk
Outdated
@@ -92,6 +90,9 @@ ${IMAGE_FILE} $(REPORT_FILE): $(IMAGES) $(SYSTEM_FILE) | |||
|
|||
include ${SDDF}/util/util.mk | |||
include ${SDDF}/network/components/network_components.mk | |||
# Specify how many pbufs sDDF LWIP library requires for all clients | |||
SDDF_LWIP_NUM_BUFFS=512 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BUFFS with two Fs looks funny. Plus (a thought) you may want a config header for this value (in lwipopts.h
perhaps) instead of a Make variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the spelling.
I agree that having this constant in the makefile not being the best option, but I am at a bit of a loss as to where to put it otherwise.
I really don't think it should be in lwipopts.h
, since that is an LWIP specific configuration file used by all of LWIP. It seems strange to make users put an sDDF library constant in their LWIP config file, plus the sDDF LWIP library does not actually explicitly use lwipopts.h
, and it only depends on it implicitly via a different LWIP include.
I am also hesitant to make another config file just for this constant, since we already have config files for the other sDDF subsystems, and this one constant is the only thing required by the library. It can also be deduced from other sDDF queue information, but since each user of the library needs to define it separately, I am not sure how to incorporate it into the build system neatly...
Since we haven't finished finalising our solution as to how this library will be rebuilt for each user, I was thinking of holding off on having a definitive solution until after that is solved.
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
Signed-off-by: Courtney Darville <[email protected]>
The changes in this PR create a reusable interfacing library for sDDF networking and LWIP. The generic components of
lwip.c
have been moved into a separate library filelib_sddf_lwip.c
and may now be used by any microkit component that wishes to use sDDF networking and LWIP. These functionalities are largely:Currently the library (of one c file) is built as a static archive using a makefile snippet. This implicitly relies on some standard library functions such as
memcpy
being available. Although the current makefile snippet works fine for the echo server example, there are two issues with it going forward:lwipopts.h
config file, as well as theSDDF_LWIP_NUM_BUFFS
constant which determines how much memory needs to be allocated for pbufs (currently passed in as a build time parameter).This library has also been integrated in LionsOS for both NFS and micropython components, and works successfully. Although since the above two issues have not yet been resolved, the library object file is made in each of the makefiles of micropython and NFS, rather than in the main Kitty and Webserver makefiles themselves.