arduino-libraries / Ethernet

Ethernet Library for Arduino
http://arduino.cc/
259 stars 264 forks source link

icmpPing missing member(functions) #105

Open orichienal opened 5 years ago

orichienal commented 5 years ago

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

Rotzbua commented 5 years ago

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

orichienal commented 5 years ago

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 commented 4 years ago

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?

masterx1981 commented 3 years ago

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 commented 3 years ago

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

Rotzbua commented 3 years ago

When it will be available?

When you have done the work 🙄

semaka commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Hi,     thanks a lot for your work. If I can do anything like, testing, reporducing, discuss about this problem I'll do with a great please in the limit of my knowledge. I'll be happy to help if I could.     Best regards,    Really appreciate you effort,    Gerard

masterx1981 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

masterx1981 commented 3 years ago

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

semaka commented 3 years ago

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

masterx1981 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

You have to include

include

include

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

semaka commented 3 years ago

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 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 commented 3 years ago

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 commented 3 years ago

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 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 commented 3 years ago

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

semaka commented 3 years ago

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

include

include

gitneko commented 3 years ago

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 commented 3 years ago

Hello @masterx1981

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

Thanks Felix

ghost commented 3 years ago

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

felmue commented 3 years ago

Hello @Josua2012

it's in masterx1981 github repo.

Thanks Felix

ghost commented 3 years ago

Thank you very much @felmue

bentzle3 commented 1 year ago

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