expose ping function to lua

[lepass7]

Missing feature

ping function

Justification

ping function will help the developer to know if there is connectivity to web and search for other devices in local network

Workarounds

I am currently using net module and doing post requests

[NicolSpies]

This would be helpful, I currently use net.dns.resolve() with a return IP address as a pseudo ping to check web connectivity. I use the MQTT server URL as the "pinged" address to in effect not only confirm general web connectivity but connectivity to the IP in play.

With the above said, to use your suggested ping function, I would have to do a dns resolve to get the IP first but I will only do it to confirm that the IP has not changed. I also assume the response size and speed from a ping will be smaller and faster when compared to a dns resolve.

[marcelstoer]

Given that you didn't ask for an ICMP ping implementation I'd vote to close this.

know if there is connectivity to web

There are several alternatives available in the firmware (plain socket, net.dns.resolve, http.request(url, "HEAD"...).

search for other devices in local network

In order to properly search you'd need more than just a ping anyway.

[NicolSpies]

I agree, we need protection against a "Ping of Death".

[wolfgangr]

May I propose to reopen the issue, please?

I hope that ping from Lua Land may be one of the first windfall results from my dealing with ethernet driver implementation (issue #1725).

• there is a app/lwip/app/ping.c and a app/include/lwip/app/ping.h in nodemcu-firmware
• there is a tutorial (by 'smcl' - knwon here?) http://blog.mclemon.io/esp8266-contributing-to-the-nodemcu-ecosystem with accompanying code https://github.com/smcl/nodemcu-firmware/ with an code example for surfacing ping to lua.

The promise is

> test.ping("google.com", 3, ping_recv)
    > 32 bytes from 216.58.214.206: icmp_seq = 1, ttl=53, time=14 m

and although I agree that there are other ways of network diagnose, people (including me) will like it.

However, smcl patches ping.c and ping.h to get the ttl and time fields. I consider those as essential parts of ping and am in favour of that. However, patching data structures breaks compatibility, which in turn provided me already with a great debugging learning example :~~| .

So we might

• keep a second interface like ping2, increasing maintenance
• don't bother breaking the interface if nobody uses ist
• change the interface and its users if we know about them
• leave out the ttl and the time fields in the output

I would ask the guardians of the code for a proposal hwo to proceed.
Without further guidance, I would go for the safe way and keep a parallel ping.

[wolfgangr]

ping pings :-)

=test.ping("www.google.de",2)
=test.ping("www.google.de",2)
> 32 bytes from 172.217.20.195, icmp_seq=25 ttl=53 time=37ms
32 bytes from 172.217.20.195, icmp_seq=26 ttl=53 time=38ms
ping 2, timeout 0, total payload 64 bytes, 1946 ms
=test.ping("192.168.0.50",2)
=test.ping("192.168.0.50",2)
> 32 bytes from 192.168.0.50, icmp_seq=27 ttl=62 time=5ms
32 bytes from 192.168.0.50, icmp_seq=28 ttl=62 time=3ms
ping 2, timeout 0, total payload 64 bytes, 1976 ms

I haven't yet decided on a place in namespace.
So i'd wait with a pull request until my other network management stuff settles.
Maybe I'd include it there.

[wolfgangr]

Following are the changes in the ping.h

...:~/test/esp-SDK/nodemcu-firmware/app/include/lwip/app$ diff ping.h ping2.h
77a78
>         uint8  ttl;

and ping.c files:

...:~/test/esp-SDK/nodemcu-firmware/app/lwip/app$ diff ping.c ping2.c
59c59
< #include "lwip/app/ping.h"
---
> #include "lwip/app/ping2.h"
167c167,170
<                                 pingmsg->ping_opt->recv_function(pingmsg->ping_opt,(void*) &pingresp);
---
>                                 pingresp.ttl = iphdr->_ttl;
>                                 pingmsg->ping_opt->recv_function(pingmsg,(void*) &pingresp);
> //                              pingmsg->ping_opt->recv_function(pingmsg->ping_opt, (void*) &pingresp);
> 

For reference, these are the complete definitions fof the relevant structs in ping2.h:

struct ping_option{
        uint32 count;
        uint32 ip;
        uint32 coarse_time;
        ping_recv_function recv_function;
        ping_sent_function sent_function;
        void* reverse;
};

struct ping_msg{
        struct ping_option *ping_opt;
        struct raw_pcb *ping_pcb;
        uint32 ping_start;
        uint32 ping_sent;
        uint32 timeout_count;
        uint32 max_count;
        uint32 sent_count;
        uint32 coarse_time;
};

struct ping_resp{
        uint32 total_count;
        uint32 resp_time;
        uint32 seqno;
        uint32 timeout_count;
        uint32 bytes;
        uint32 total_bytes;
        uint32 total_time;
        uint8  ttl;
        sint8  ping_err;
};

From my (still limited) understanding of C, the addition of the ttl-Field to ping_resp would not hurt anybody, as long as they rely on the symbolic refernces, right?

However, the change from pingmsg->ping_opt to pingmsg as pointer to handed over to the registered function upon ping received severly brakes the interface.

Would it be a solution to include a field like parent or msg to ping_opt where we store a pointer to the msg? While this still may look awkward, it would not brake interfaces. An appliation expecting the old behaviour would still receive the ping_opt it expects. An application aware of the changes could load the msg by this pointer and thus retrieve the ping time value.

[wolfgangr]

ok, just hacked this backreference, looks like it works.
Still some causes for reboot i have to catch

[wolfgangr]

OK, I catched the reboot issue on ping when WIFI down.

There are still some quirks with converting illegal IP numbers, but this is done by lwIP itself, and I do not dare to fiddle with this.

If somebody wants to try before the namespace question is settled:
https://github.com/wolfgangr/nodemcu-firmware/tree/wolfgangr-ping

file added:

/app/modules/test.c

files changed:

/app/include/lwip/app/ping.h
/app/include/user_modules.h
/app/lwip/app/ping.c

Presumably it even could handle callbacks on pings received, but I haven't tested.
Find some docu here (bottom qarter of the tutorial):
http://blog.mclemon.io/esp8266-contributing-to-the-nodemcu-ecosystem

[devyte]

I think this is a great addition, good job! I suggest making it net.ping().

[marcelstoer]

Good stuff.

So i'd wait with a pull request until my other network management stuff settles.

We prefer small isolate PRs for isolated features rather than one big blob.

I suggest making it net.ping().

👍

[wolfgangr]

suggest making it net.ping()

OK, sounds good.

We prefer small isolate PRs for isolated features

So would I. But I'd like to keep the goal in mind, which was an ethernet driver.
So, can I later have net.netif() or net.if() in a different file?

Do file names and module names have any other connection rather than by convention?
How is the source tree to be organized? So where are files supposed to live?

I simply could keep the file in place and rename it to nodemcu-firmware/app/modules/ping.c .
But I'd consider to open a network related subfolder?
Should I? And if so, where?

After obvious cleanup of tutorial scrap, this is what the function map definition might look like

// Module function map
static const LUA_REG_TYPE test_map[] = {
  { LSTRKEY( "ping" ),             LFUNCVAL( test_ping ) },
  { LSTRKEY( "__metatable" ),      LROVAL( test_map ) },
  { LNILKEY, LNILVAL }
};

// Define an empty test function 
int luaopen_test( lua_State *L ) {
  return 0;
}

NODEMCU_MODULE(TEST, "test", test_map, luaopen_test);

OK, change all test to net is the second obvious thing to do.
Ehmm - nearly all.
Should I have a luaopen_test function? Right now, I only know that omitting it breaks build...
Or is it just fair to leave room for others doing future improvements ;-)

And I think I 'm supposed to supply Documentation - at least in some later PR to come.
Where? How? (mmh, I remember some contribution guide...should read that again this night...)

And how can I merge module maps across files?
Can I do so at all?
How is the module map thing processed?
Is there any documentation? Or at least a pointer to the relavant source?

I see there is a subfolder lua_modules/*.
I would even like to include lua code under the name space prefix as well.
Just think of net.traceroute() ....., or parsing the netif interface list in lua, keeping the C binding layer as thin as possible.

How is all this handled in user_modules.h?
Maybe this goes towards a module dependency tree, but I can memorize that there were a discussion somewhere on the internet on the question of flat vs. tree like module dependencies, right?