Skip to content
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

Add Support for Network.ping() #156

Merged
merged 1 commit into from
Apr 13, 2014
Merged

Conversation

bkobkobko
Copy link
Contributor

We might want to have some discussion about the while loop that polls for either a report struct or timeout. If the user asks for 5000 pings, especially for a host that is unreachable, the Spark loop could timeout but I am not sure we want to call the Spark loop here.

Here's the test program:

include "application.h"

IPAddress google(74,125,225,52);
IPAddress yahoo(98,139,180,149);
IPAddress spark(62,116,130,8);

void setup() {
Serial.begin(9600);
delay(2000);
}

void loop() {
Serial.print("G: ");
Serial.println(Network.ping(google));
Serial.print(" Y: ");
Serial.println(Network.ping(yahoo));
Serial.print(" S: ");
Serial.println(Network.ping(spark));
Serial.print(" Fail: ");
Serial.println(Network.ping(IPAddress(10,0,0,200))); // picked to fail
delay(10000);
}

@bkobkobko
Copy link
Contributor Author

If the call to netapp_ping_send() fails, then psend will be non-zero and Line 134 if (psend...) will not set the result and so result will be the zero value it was initialized to.

You could put this test before the while loop and skip waiting for the timeout, but in my testing it never goes off.

@bkobkobko
Copy link
Contributor Author

One other interesting thing I just discovered! If you call Network.ping(...) as the first think in loop() and you have not called SPARK_WLAN_Loop() either directly by going around loop or via delay(), the first pings all return 0 responses. Looking at main.cpp I see that SPARK_WLAN_Loop is called at the top of the while(1) loop before user setup() and loop(). If I add a call to SPARK_WLAN_Loop() between user setup() and loop() in main, the first pings all work correctly.

The async callback for ping is setup in SPARK_WLAN_Setup(), not _Loop() so that does not make a lot of sense to me. I see the pings going out my router but it is hard to know what's coming back without a more invasive test setup.

@davids5
Copy link

davids5 commented Mar 28, 2014

Try testing the result of psend = netapp_ping_send(&pingIPAddr, (unsigned long)nTries, pingSize, pingTimeout); what is it?

@bkobkobko
Copy link
Contributor Author

OK, so psend is 0 meaning success but simply putting any code between the netapp_ping_send call and the next line to call millis(); fixes the problem. I tried putting int foo = 12; there and that worked fine, as did NULL;

It must be a timing thing, but I am not seeing it. So I moved the default assignment of result and it works fine at the start of loop now.

@davids5
Copy link

davids5 commented Mar 28, 2014

ehhhh optimization bug? Skype?

@davids5
Copy link

davids5 commented Mar 28, 2014

The timing difference of one load like foo=12 in in the sub uS range.
That kind of strangeness is not good. Sounds like a stack crash or overwrite.
try putting a long x = 0xaa55cc33 at entry and in your calling routine. then check it on exit and panic if not == 0xaa55cc33....

@bkobkobko
Copy link
Contributor Author

Interesting! I added a uint32_t topstack = 0xdeadbeef; and check it at the end and that works fine. Then I comment out the new code still passes! Then I edit out the commented out code--fails first time again! What the ????

OK, let's try something else: Once I see a failure after downloading, I hit the reset button 10 times noting if the first pings succeed or fail and they all succeed! The only one that failed was immediately after dfu-util.

Then I pulled the power to the core a bunch of times and again, it never failed.

It had something to do with downloading over dfu-util. I say "had" because now I can't make it fail no matter what I do. I just flashed with dfu-util 10 times and cannot make it fail.

@davids5
Copy link

davids5 commented Mar 28, 2014

Yep you are on to something… I have seen red flashes on the reset after the DFU is run implying the code cannot reach the internet.

This seemed to introduced in the PR that in common-lib that wrapped disable_irq() and __enable_irq();

You could try backing out the disable and enable and see if it changes the behavior, then we need to determine what the root cause is.

So at setup or sooner Save socket_active_status and print it when can. Also what is the state of the IO in the transition from DFU to run.

void set_socket_active_status(long Sd, long Status)

{

        DEBUG("Sd=%d, Status %s",Sd, Status == SOCKET_STATUS_ACTIVE ?  "SOCKET_STATUS_ACTIVE" : "SOCKET_STATUS_IACTIVE");

            if(M_IS_VALID_SD(Sd) && M_IS_VALID_STATUS(Status))

            {

                    uint32_t is = __get_PRIMASK();

              __disable_irq();

                        socket_active_status &= ~(1 << Sd);      /* clean socket's mask */

                        socket_active_status |= (Status << Sd); /* set new socket's mask */

              if ((is & 1) == 0) {

                  __enable_irq();

            }

}

}

@towynlin
Copy link
Contributor

Good discussion here. I've seen occasional unexplained failures immediately after using dfu-util as well, usually manifesting, like David said, as 3 red flashes on the first run after DFU. As far as I know this behavior has always been present. Some binaries seem to manifest it more than others.

In any case, that's all sort of off topic of this particular pull request, which looks good and I'm merging now. 👍 Thanks Brian!

towynlin pushed a commit that referenced this pull request Apr 13, 2014
Add Support for Network.ping()
@towynlin towynlin merged commit c4b4bfe into particle-iot:master Apr 13, 2014
@bkobkobko bkobkobko deleted the NetworkPing branch April 15, 2014 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants