Skip to content
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

lwip2: lighter implementation #3362

Closed
wants to merge 88 commits into from
Closed

lwip2: lighter implementation #3362

wants to merge 88 commits into from

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Jun 19, 2017

This PR is replaced by #3783

full sources are separate and available as a submodule
included in this PR: include/ and liblwip2.a + Makefile for pull&rebuild

I guess this is the way @igrr imports his cloned axtls, so arduino/esp8266 is lighter.

aside from this lwip2 implementation gets more genericity for other esp8266 projects.

@d-a-v d-a-v mentioned this pull request Jul 15, 2017
@d-a-v d-a-v mentioned this pull request Aug 30, 2017
@devyte
Copy link
Collaborator

devyte commented Sep 9, 2017

@igrr are we waiting until after the next release to merge this, or do we want it in the next release?

@igrr
Copy link
Member

igrr commented Sep 12, 2017

Yes, we're waiting until after the next release.

igrr and others added 20 commits September 29, 2017 11:31
Includes two PRs:

- igrr/axtls-8266#46 by @earlephilhower:
  Move debug strings from RAM to Flash

- igrr/axtls-8266#50:
  Fix memory leak in ssl_ext_host_name
Every assert() includes a __FILE__ constant string to RODATA which
can be quite large as it includes the complete path as well as
the filename.

Move that string into PMEM, and update the postmortem to work with
either PMEM or RAM strings for dumping abort/assert/exception
information.
The ax_port_malloc, ax_port_calloc, ax_port_realloc, and ax_port_free
functions in WiFiClientSecure are not actually used by the AXTLS library.
It's directly using the library routines, and these function are never
used.  Remove these dead bits of code to make the axtls operation clearer.
Custom manifest for @platformio which instructs PIO Build System to do not archive library's object files. See 

- esp8266#3321 (comment)
- http://docs.platformio.org/en/latest/librarymanager/config.html#libarchive
UMM debugging strings are normally placed in RODATA, which uses up scarse
memory.  Move them to PROGMEM and use macros to replace printf with a
version that can handle ROM strings.
The extension -> MIME type routine uses lots of constant strings which end
up in the RODATA segment of RAM.  Refactor the comparison to use a table of
strings stored in PMMEM instead, freeing ~370 bytes for the heap.
The Arduino IDE requires the use of a tab separator between the name and identifier. Without this tab the keyword is not highlighted.

Reference: https://github.com/arduino/Arduino/wiki/Arduino-IDE-1.5:-Library-specification#keywords
The RST spec does not appear to believe in intra-cell line continuation as appeared to be expected by the original author.  Widen columns and unbreak words to fix output.
realloc() is called with newSize > 0 (at least 16), so newbuffer==0 means the old memory was not deallocated. Therefore, the pointer should still point to the old buffer. This change should resolve issue esp8266#3516.
@d-a-v
Copy link
Collaborator Author

d-a-v commented Oct 25, 2017

ram + flash footprint is now better than with lwip1.4.
Below the footprint variation with one of my sketches: The two last columns show the latest commits benefits.
mainly: I had forgotten to disable lwip internal asserts :-/

                1.4             v2              -assert         optsync+dns
sketch          373912          380616          367684          367236
sketch%         74              76              73              73
ram             40396           47940           40932           40020
ram%            49              58              49              48
heap            33240           25360           32872           33280

@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 1, 2017

@igrr @devyte @davisonja At this point, and

  • because people are beginning to be aware that lwip2 can solve some old issues and some of them already use it,
  • because this PR cannot not be merged as-is (gitk shows real tube map due to repaired mistakes and also to my inability to stay clean in the long run with git and the multiple branches I'm dealing with - I lost liblwip2.a the other day, and I regularly lose myself in suddenly appearing head-detached branch :),
  • because this PR is really small (it's one makefile, one line in the main ldscript, the include directory+lib which are however auto-generated, and a submodule ref),
  • because memory footprint seems equivalent to lwip1.4's,
  • because lwip2 needs more serious testing
  • because I don't want to and will not advertise my own repo

What do you think if

  • I include in here the regenerated boards.txt from boards.txt generator #3722 so it is tested too (and has lwip2+debug for all known boards)
  • lwip2 is advertised in issues where relevant with an easy step-by-step setup doc
  • and of course I keep up syncing with master so testers can always have master+lwip2

@igrr
Copy link
Member

igrr commented Nov 1, 2017 via email

@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 1, 2017

To master, I was not asking for that much :) Did I understand well ?
The boards.txt issue must be solved first so lwip2 can be available in menus, but still along with the former one that we must have around. @davisonja what about #3722 and ldscripts ?
Or should we provide a generated boards.txt at first ?
Or simply keep the modified master's boards.txt from this PR ?

@davisonja
Copy link

Because the ldscripts need to match I would vote to include a generated boards.txt, though in practice since it should look the same as the modified version in this pr its probably just as simple to leave the current one. I've got some queries about the current script, but been tied up with new child, hopefully can post them tonight 😊

@devyte
Copy link
Collaborator

devyte commented Nov 2, 2017

@d-a-v if this PR is superseded, should it be closed?
Similar question for PR #3206

@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 2, 2017

closing in favour of #3783

@d-a-v d-a-v closed this Nov 2, 2017
@igrr
Copy link
Member

igrr commented Nov 2, 2017

@d-a-v I did mean master. Since lwip v2 is optional, it shouldn't be any harm, and hopefully will help folks who run into lwip related issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.