-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
Netump Initial commit #7527
Netump Initial commit #7527
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 |
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.
Works for me, nice tool for debugging network issues or protocols,
Thanks !
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p.printf("%d.%d.%d.%d")?
/* | ||
NetdumpIP.h | ||
|
||
Created on: 18 mei 2019 |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
* master: (299 commits) Fix error message typo (esp8266#7581) Update certs-from-mozilla.py (esp8266#7578) Update DigestAuthorization.ino (Simple example update) (esp8266#7579) Fix gzip+signed OTA error (esp8266#7577) Properly replace toolchain in PlatformIO CI script (esp8266#7580) Update certs-from-mozilla.py (esp8266#7573) Fixup weird combination of oneline/multi line comments (esp8266#7566) Reduce codesize of setOutputPower (esp8266#7572) Fix typos in tests Force gcc inlining, use same style for getCycleCount as for getCpuFreqMHz. Even more concise #if form. Inline, fewer LOC, remove redundant definition in cpp. Netump Initial commit (esp8266#7527) Delete owner field (esp8266#7563) Avoid float-double-conversion (esp8266#7559) Use direct member initialization instead of ctr initialisation (esp8266#7556) Add CI test for eboot build (esp8266#7546) getCpuFreqMHz(): fix when F_CPU is not defined (esp8266#7554) emulation-on-host makefile update, allowing to pass more options (esp8266#7552) add sdk options to "generic esp8285 module" (esp8266#7550) ...
This is a continuation from #6518
To ease tracking I first commit the same code, updates come with next commits.