Netump Initial commit#7527
Netump Initial commit#7527earlephilhower merged 6 commits intoesp8266:masterfrom hreintke:NetdumpLibrary
Conversation
|
Reply to comments @d-a-v @devyte On default constructor/deconstructor being generated by compiler.
Done
Needed -> Not done
These are netdump specific. wrapping lwip calls only complicates the code.
NetdumpIP is created because lwip callback ans as such netdump always will have both IPv4 ans IPv6. Regardless of the lwip seting.
Constructor needed -> not done
Done
Do not understand your comment.
Done
Done |
d-a-v
left a comment
d-a-v
left a comment
There was a problem hiding this comment.
Works for me, nice tool for debugging network issues or protocols,
Thanks !
earlephilhower
left a comment
earlephilhower
left a comment
There was a problem hiding this comment.
This is very awesome!
Nothing that would stop me merging, but a few points you may want to look at for your own edification.
|
|
||
| bool NetdumpIP::fromString4(const char *address) | ||
| { | ||
| // TODO: (IPv4) add support for "a", "a.b", "a.b.c" formats |
There was a problem hiding this comment.
Why not int a,b,c,d; int elems = scanf("%d.%d.%d.%d", &a, &b, &c, &d); ...?
| @@ -0,0 +1,9 @@ | |||
| name=NetDump | |||
| version=2 | |||
| author=Herman Reintke | |||
There was a problem hiding this comment.
Your call, but normally there's an email or something else to direct people (@hreintke even) in these fields.
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
Might want to add the new KEYWORD1/2s. Not a big deal.
| uint32_t acc = 0; // Accumulator | ||
| int dots = 0, doubledots = -1; | ||
|
|
||
| while (*address) |
There was a problem hiding this comment.
Same as IPV4, but maybe it's not so easy with potential empty slots (i.e. fe:aa2::::...)
| } | ||
| return n; | ||
| } | ||
| for (int i = 0; i < 4; i++) |
There was a problem hiding this comment.
p.printf("%d.%d.%d.%d")?
| /* | ||
| NetdumpIP.h | ||
|
|
||
| Created on: 18 mei 2019 |
There was a problem hiding this comment.
Would you like to include the GPL and your full name here?
| { | ||
| for (int i = 0; i < 6; i++) | ||
| { | ||
| sstr.printf_P(PSTR("%02x"), (unsigned char)data[dataIdx + i]); |
There was a problem hiding this comment.
If we're using printf_P(PSTR("%02x")), can we use it elsewhere where printf(%02x") is? Now we'll end up with copies in RODATA, TEXT, and IROM.PSTR...
|
This is too valuable to wait for minor tweaks. Merging and will open an issue pointing to some of the comments herein. |
3 participants
This is a continuation from #6518
To ease tracking I first commit the same code, updates come with next commits.