Skip to content

icmpPing missing member(functions) #105

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

Open
orichienal opened this issue Jul 14, 2019 · 33 comments
Open

icmpPing missing member(functions) #105

orichienal opened this issue Jul 14, 2019 · 33 comments
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project

Comments

@orichienal
Copy link

orichienal commented Jul 14, 2019

hi Guys

The new Ethernet 2.0 library is very different from the old one.
Unfortunately, some functions have been lost.

I have to use Ethernet2.0 and also ICMPPing from BlakeFoster.

So far, he has not responded and in the Arduino forum, no one could help me.

But I am not alone with this problem.

I hope someone can spread a solution or approach that makes it possible to ping from an Arduino.

Here are the error messages:

C:\arduino-1.8.8\portable\sketchbook\libraries\icmp_ping\ICMPPing.cpp: In member function 'Status ICMPPing::sendEchoRequest(const IPAddress&, const ICMPEcho&)':

C:\arduino-1.8.8\portable\sketchbook\libraries\icmp_ping\ICMPPing.cpp:184:11: error: 'class W5100Class' has no member named 'send_data_processing'

     W5100.send_data_processing(_socket, serialized, sizeof(ICMPEcho));

           ^

C:\arduino-1.8.8\portable\sketchbook\libraries\icmp_ping\ICMPPing.cpp: In member function 'void ICMPPing::receiveEchoReply(const ICMPEcho&, const IPAddress&, ICMPEchoReply&)':

C:\arduino-1.8.8\portable\sketchbook\libraries\icmp_ping\ICMPPing.cpp:207:19: error: 'class W5100Class' has no member named 'getRXReceivedSize'

         if (W5100.getRXReceivedSize(_socket) < 1)

                   ^

C:\arduino-1.8.8\portable\sketchbook\libraries\icmp_ping\ICMPPing.cpp:219:9: error: 'class W5100Class' has no member named 'read_data'

   W5100.read_data(_socket, (uint16_t) buffer, ipHeader, sizeof(ipHeader));

         ^

C:\arduino-1.8.8\portable\sketchbook\libraries\icmp_ping\ICMPPing.cpp:229:9: error: 'class W5100Class' has no member named 'read_data'

   W5100.read_data(_socket, (uint16_t) buffer, serialized, dataLen);

Thank you very much in advance.
orichienal

@orichienal orichienal changed the title icmpPing icmpPing missing member(functions) Jul 14, 2019
@Rotzbua
Copy link
Contributor

Rotzbua commented Jul 14, 2019

No problem with this library. You have to open an issue at https://github.com/BlakeFoster/Arduino-Ping

@orichienal
Copy link
Author

i opened some time ago an issue at https://github.com/BlakeFoster/Arduino-Ping

But I think the owner will not be able to help me in a short time

I decided to help myself and that's why I'm looking for help here

I was hoping someone could help me understand the new functions and use them the way they did the old ones

these are the old functions and now i want to use some of the new functions to do the same
send_data_processing
getRXReceivedSize
read_data

and i hope someone could help me, i am not the only one who want to ping from arduino.

@ulterify
Copy link

ulterify commented Sep 21, 2020

I've looked into this, and found that the way the Ethernet library is currently structured makes it impossible to extend to use additional protocols such as ICMP ping.

I can try to fix this, but there's a philosophical debate to be had first: is the EthernetClass class meant to be used by user-supplied classes, and should you be able to extend the library in that way, or is it meant to be closed to that sort of thing.

Both have their advantages - opening it up to extensibility has the obvious advantage of making things like an additional ICMPPing library possible again, but introduces the burden of keeping a much larger API surface stable.

The concrete issue:

The ICMPPing library used to use functions such as send_data_processing. This function is now superseded by the static function write_data, which is not reachable from outside.
Now there is a function called EthernetClass::socketBufferData which is a thin wrapper around write_data, but that function is declared private. The EthernetUDP class calls this function, and it can do so because it is declared a friend of EthernetClass. This illustrates the current pattern in the library: implement supporting functions for each protocol directly in the EthernetClass class as private, declare any protocol implementation as a friend.

Two possible solutions, depending on the philosophy we want to have:

  1. Make it possible to use some formerly 'internal' functions in the Ethernet class and use its functionality, without having to declare a friend (cleaning these up to provide a clean API)
  2. Integrate support for ICMP into the Ethernet library, the way this is done for TCP and UDP: by adding the necessary supporting functions into the EthernetClass class, declaring EthernetICMP a friend of the ethernet class, and implementing an EthernetICMP.cpp

Either way, I'm willing to put in the work.
I have a slight preference for option 2, since I think ICMP support should be an integral part of any Ethernet library.

Any thoughts?

  • Edited: inheritance not playing a role here.

@masterx1981
Copy link

I'm interested too in this. By now i've added the missing functions in w5100.h/cpp for have the icmp back working...

@semaka
Copy link

semaka commented Jan 16, 2021

I'm also interested in this, it should work again, let's implement the best solution. When it will be available?

@Rotzbua
Copy link
Contributor

Rotzbua commented Jan 16, 2021

When it will be available?

When you have done the work 🙄

@semaka
Copy link

semaka commented Jan 17, 2021

I didn't do anything of this work, but as soon as it is not working and nothing is intended to be corrected it should be delete it and do not keep the script on github not functionally. Maybe somebody else will do another script working. Or do you think that github is a place of non-working scripts?

@masterx1981
Copy link

I can try to integrate the ping function directly in the library (as proposed by @ulterify, option 2, as i'm agree that icmp is a base eth protocol and must be directly supported) but i not know if the main library will be updated. Almost one month ago I've made some mods for fix a bug on registry address definition on w5500, and i not ever know if someone noticed the pull request... and i see many other pull requests ignored.
By now i can use the icmpping library as i've edited the ethernet library adding the missing functions, not the best but not lost too much time and it works.

@orichienal
Copy link
Author

I've been looking for a solution to ping the Ethernet2.0 library for so long.
You seem to have adapted the 2.0 so that old ICMPPing from BlakeFoster works.
Is there a way to get this version? I tried the fork of masterx1981, but unfortunately without success.
I have found a lot of people with this problem on the net and that would be a great solution now.
I need it urgently. I don't understand how such an elementary function cannot be integrated.

I would be very happy to receive feedback and help with a very long-lasting problem.

I have already been active in several forums and would like to link to this solution here, many would be helped.

@masterx1981
Copy link

The version that I have on github doesn't include the icmp support, as my implementation is far from optimal. The github version only fixes some problems of the Ethernet library with W5500. When i have time i try to merge the icmpping library with the ethernet library.

@orichienal
Copy link
Author

I tried to transfer the "missing" functions from the older version to the new one myself.
But that doesn't really work, WireShark is saying that ICMP is not correct.
Nothing comes back to the Arduino either.
And then at some point it crashes and the Arduino restarts.
Maybe you could send me your version even if it's not optimal.
Then maybe I understand what I did wrong.

@masterx1981
Copy link

masterx1981 commented Feb 18, 2021

I've managed to include the https://github.com/BlakeFoster/Arduino-Ping functions in the ethernet library (using ethernet function and not readding the old ones - the new libraries, despite a name change, are almost identical to the old ones), but seem to cause some memory corruption. The corruption was already there with Blake's library untouched, and i've noticed it when my code reached ~90kb flash size (but with more probability it's related to the memory usage).
I need to investigate a bit more on what's causing it. When i've found the cause, i update my git library

@semaka
Copy link

semaka commented Feb 19, 2021 via email

@masterx1981
Copy link

masterx1981 commented Feb 20, 2021

I've updated my GIT repository. It's a simple copy/paste of the BlakeFoster work.
The memory bug was caused by another thing in my code, so nothing to worry
I tested it in sync mode with the following code:

const SOCKET pingSocket = 0;
EthernetICMPPing pingip(pingSocket, (uint8_t)random(0, 255));
pingip.setTimeout(timeout * 1000);
EthernetICMPEchoReply echoReply = pingip(IPAddress, 1);
if (echoReply.status == SUCCESS) DOSOMETHING

@semaka
Copy link

semaka commented Mar 19, 2021

Is it working for you? I still have error because this library ICMPPing makes reference to members not declared in Ethernet for my W5100. I have following errors:
ICMPPing.cpp:184:11: error: 'class W5100Class' has no member named 'send_data_processing'
ICMPPing.cpp:207:19: error: 'class W5100Class' has no member named 'getRXReceivedSize'
ICMPPing.cpp:219:9: error: 'class W5100Class' has no member named 'read_data'

Thank you for any help in this.

@masterx1981
Copy link

Is it working for you? I still have error because this library ICMPPing makes reference to members not declared in Ethernet for my W5100. I have following errors:
ICMPPing.cpp:184:11: error: 'class W5100Class' has no member named 'send_data_processing'
ICMPPing.cpp:207:19: error: 'class W5100Class' has no member named 'getRXReceivedSize'
ICMPPing.cpp:219:9: error: 'class W5100Class' has no member named 'read_data'

Thank you for any help in this.

Have you tried using my version?

@semaka
Copy link

semaka commented Mar 19, 2021

Yes I tried, especially with Ethernet library from your git, but I still see the same errors.

@masterx1981
Copy link

If you are using my modified ethernet library you not have to add the icmpping library, as it's already embedded on it.

@semaka
Copy link

semaka commented Mar 20, 2021

do you also have a complete code with setup and loop to test this by using just your library?

@masterx1981
Copy link

const SOCKET pingSocket = 0;
EthernetICMPPing pingip(pingSocket, (uint8_t)random(0, 255));
pingip.setTimeout(timeout * 1000);
EthernetICMPEchoReply echoReply = pingip(IPAddress, 1);
if (echoReply.status == SUCCESS) DOSOMETHING

You can use it exactely the same as original icmpping library, use this as template how to access the icmp functions inside the library. Sorry but by now i have no sample code

@semaka
Copy link

semaka commented Mar 20, 2021

OK, here is first error by using just your library ethernet. It seems that SOCKET is missing in your ethernet which was present in icmp_ping.
PingNew:22:7: error: 'SOCKET' does not name a type; did you mean 'SCK'?

@masterx1981
Copy link

You have to include
#include <EthernetIcmp.h>
#include <Ethernet.h>

Maybe open an issue un my git repository so that we not add unnecessary things here.

@semaka
Copy link

semaka commented Mar 21, 2021

Thank a lot. It is not necessary to open an issue anymore, IT WORKS, with few adjustments I did it. I made the test and it works great. By the way, the correct name of library is #include <EthernetICMP.h>
Congratulation, you did a nice job for so long wait, I'd like to understand more and to develop things like you in one day.

@masterx1981
Copy link

Sorry, the phone have corrected the capital letters. I've only "pasted" 2 libraries together. Hope that some day someone that really know how to code, fix this thing in a proper way in the official arduino ethernet library...

@gitneko
Copy link

gitneko commented Apr 11, 2021

Thank a lot. It is not necessary to open an issue anymore, IT WORKS, with few adjustments I did it. I made the test and it works great. By the way, the correct name of library is #include <EthernetICMP.h>
Congratulation, you did a nice job for so long wait, I'd like to understand more and to develop things like you in one day.

Could you please post which adjustments you had to do?

Currently the fork of @masterx1981 with ICMP does not work, because there are undefined references (according to the compiler error output).

libraries\Ethernet\EthernetICMP.cpp.o: In function `EthernetICMPPing::operator()(arduino::IPAddress const&, int, EthernetICMPEchoReply&)':
C:\Program Files (x86)\Arduino\libraries\Ethernet\src/EthernetICMP.cpp:175: undefined reference to `EthernetICMPPing::sendEchoRequest(arduino::IPAddress const&, EthernetICMPEcho const&)'
C:\Program Files (x86)\Arduino\libraries\Ethernet\src/EthernetICMP.cpp:180: undefined reference to `EthernetICMPPing::receiveEchoReply(EthernetICMPEcho const&, arduino::IPAddress const&, EthernetICMPEchoReply&)'
collect2.exe: error: ld returned 1 exit status
exit status 1

@masterx1981
Copy link

Uhm, on my arduino environment it compiles well. Need to check.

@semaka
Copy link

semaka commented Apr 12, 2021

the most important thing is to have the correct libraries installed and included as my list below. I also tested and it works great, sometimes I see a delay and even if the device is connected ping does not respond but for this I asked 3 consecutive times to be sure.
#include <SPI.h>
#include <EthernetICMP.h>
#include <Ethernet.h>

@gitneko
Copy link

gitneko commented Apr 12, 2021

Uhm, on my arduino environment it compiles well. Need to check.

Yup, it suddenly works too. I don't know why or how. But I haven't tested the function yet.

@felmue
Copy link

felmue commented Jun 1, 2021

Hello @masterx1981

I was looking for an ESP32 / W5500 ping library today. You've saved my day. Works like a charm.

Thanks
Felix

@ghost
Copy link

ghost commented Oct 31, 2021

Hi,
Where can I download the version of the Ethernet library with pinging support?
Thanks.
Regards,
Jose

@felmue
Copy link

felmue commented Oct 31, 2021

Hello @Josua2012

it's in masterx1981 github repo.

Thanks
Felix

@ghost
Copy link

ghost commented Nov 1, 2021

Thank you very much @felmue

@bentzle3
Copy link

bentzle3 commented Dec 30, 2022

I have the same issue @gitneko had prior to it suddenly starting to work:

/var/folders/39/rdsn92s55tjgq_tzvrbtj3_m0000gn/T//cc0db78l.ltrans0.ltrans.o: In function `operator()':
/Users/brandon/Library/Mobile Documents/com~apple~CloudDocs/Documents/code/Ardurino/libraries/Ethernet/src/EthernetICMP.cpp:180: undefined reference to `EthernetICMPPing::sendEchoRequest(IPAddress const&, EthernetICMPEcho const&)'
/Users/brandon/Library/Mobile Documents/com~apple~CloudDocs/Documents/code/Ardurino/libraries/Ethernet/src/EthernetICMP.cpp:185: undefined reference to `EthernetICMPPing::receiveEchoReply(EthernetICMPEcho const&, IPAddress const&, EthernetICMPEchoReply&)'
collect2: error: ld returned 1 exit status

@per1234 per1234 added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project labels Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

No branches or pull requests

9 participants