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

Made Ethernet library non Blocking #1240

Open
wants to merge 2 commits into
base: master
from

Conversation

8 participants
@DavyLandman

DavyLandman commented Jan 21, 2013

Hi Guys,

I've extended the Ethernet library to make it possible to work with it in a non blocking way, which makes it easier to combine the library with handeling other task (handeling buttons, updating a screen etc) while a DHCP lease happens, or a packet is sent.

I've added non blocking variants for:

  • Dhcp::*
  • Dns::getHostbyName
  • Ethernet::begin (DHCP version)
  • Ethernet::maintain
  • EthernetClient::connect(*)

I welcome comments on the api-style, not really glad with all the names, but what would you guys think? I also need to think about a non blocking send, since it now loops until enough memory is available, I could make a variant which actually returns the bytes it could send, but I still have to think if this is the best way to do this.

There are 2 commits in this pull request, the Ethernet library was a mix of 3 different indentation styles, so I looked at the other parts and made commit a2ca62d to first only fix the formatting of the code. Then there is a new commit which actually changes the code, commit 2ac4e2f . This is actually an import from the work I started in DavyLandman/NBEthernet where I found out that the existing Ethernet library was already half non blocking.

So when reviewing, make sure to select just the correct commit (2ac4e2f), else in the github summary will claim I changed almost everything..

Impact on the size is quite minimal (for example DnsWebClient ):

old: Binary sketch size: 13,474 bytes (of a 30,720 byte maximum)
new: Binary sketch size: 14,980 bytes (of a 30,720 byte maximum)

The increase is primarily due to keeping a blocking variant.

DavyLandman added some commits Jan 7, 2013

Imported work from DavyLandman/NBEthernet adding a non blocking varia…
…nt of most Ethernet functions.

Non blocking alternatives added for:

- Dhcp::*
- Dns::getHostbyName
- Ethernet::begin (DHCP version)
- Ethernet::maintain
- EthernetClient::connect(*)
@DavyLandman

This comment has been minimized.

DavyLandman commented Jan 30, 2013

so, a github pull request is not the way to contribute, what is the way to do this? if I look at other pull requests, almost no responses?

@cmaglie

This comment has been minimized.

Member

cmaglie commented Jan 31, 2013

Davy,
I looked into the patch, seems well done.
In general, API changes (even smaller ones) must be discussed in the developers list before being accepted.
Some questions:

  1. The increased size could be a problem for sketches that are near the 32K limit (or 16K for boards that runs on mega168), could it be avoided? (I mean by exploiting compiler optimization for unused method/functions)

  2. This patch seems to add functionality while keeping backward compatibility, is that right?

  3. Do you have some documentation about the added methods? Can you provide some examples?

@DavyLandman

This comment has been minimized.

DavyLandman commented Jan 31, 2013

@cmaglie didn't know about the developers mailing list, should I start a discussion there?

Regarding your questions:

  1. As far as I understand, compiler optimizations would work for static methods, since most additions are class methods, I would wonder how the compiler could decide to optimize those away.
  2. Yes, I've added functionality while keeping everything backwards compatible (but using the same code paths). I think this might contribute to the increase in code size.
  3. I've tried to add "documentation" to the .h files where I added a new method. But this here is an example, which uses all the new asynchronous methods. A user can mix them:
EthernetClient client;
static uint32_t next_connect;

void setup() {
    // start the serial library:
    Serial.begin(9600);
    Serial.println("Initializing Ethernet controller");
    Ethernet.initialize(mac);
    int result = 0;
    do {
        result = Ethernet.initialized();
    } while (result == 0);

    if (result == 2) {
        Serial.println("Failed to configure Ethernet using DHCP");
        while (true) ;
    }
    Serial.println("Got IP adress");
    Serial.println(Ethernet.localIP());
    next_connect = millis() + TIMEOUT;
}

static byte maintaining = 0; // DHCP maintain happening
static byte insession = 0; // a HTTP GET is in session 
static byte waiting = 0; // waiting for the connection to be established
static uint16_t bytes_read;

// crapy state machine, for real applications, use something better ;-)
void loop() {
    if (maintaining) {
        int result = Ethernet.maintainFinished();
        maintaining = result == 0;  
        if (!maintaining) {
            if (result == 1) {
                Serial.println("DHCP succeeded");
                Serial.println("Got IP adress");
                Serial.println(Ethernet.localIP());
            }
            if (result == 2) {
                Serial.println("DHCP failed");
            }
        }
        return;
    }
    else {
        maintaining = Ethernet.maintainNeeded() != DHCP_CHECK_NONE;
        if (maintaining) {
            Serial.println();
            Serial.println();
            Serial.println("DHCP timeout, getting new one");
            return;
        }
    }
    if (!insession) {
        if (!waiting && millis() >= next_connect) {
            next_connect += TIMEOUT;
            Serial.println("Starting a new connection");
            if (!client.initializeConnection("www.google.com", 80)) {
                Serial.println("initializing connection failed");
            }
            else {
                waiting = 1;
            }
        }
        if (waiting && client.connectionInitialized()) {
            if (client.connectionInitialized() == 1) {
                Serial.println("connected");
                client.println("GET /arduino HTTP/1.0");
                client.println();
                waiting = 0;
                insession = 1;
                bytes_read = 0;
            }
            else {
                waiting = 0;
                Serial.println("something went wrong with getting a connection");
            }
        }
    }
    else {
        // we have a connection, and the GET has been sent.
        // lets consume the response
        if (client.available() && bytes_read < READLIMIT) {
            char c = client.read();
            Serial.print(c);
            bytes_read++;
        }
        else if (client.available() && bytes_read == READLIMIT) {
            client.flush();
        }

        if (!client.connected()) {
            Serial.println();
            Serial.println("disconnecting.");
            client.stop();
            insession = 0;
            waiting = 0;
        }
    }
}
@bilics

This comment has been minimized.

bilics commented May 24, 2013

Hi,

Just wondering if this has been committed to the official release... Are there any plans to do so ?

Thanks!

@DavyLandman

This comment has been minimized.

DavyLandman commented May 24, 2013

I also tried to get this conversation going in the google groups developer discussion: https://groups.google.com/a/arduino.cc/d/msg/developers/vv419s8-Zw0/mUKAZAyU_MoJ

but that didn't speed things along either.

@bilics

This comment has been minimized.

bilics commented May 24, 2013

Thanks for the reply Davy - I would really like to see this in the official release.

Anyway, I will download this version and try to use it in my project now.

As far as I understand, even the initialize() / initialized() calls should be non-blocking, correct ?

@DavyLandman

This comment has been minimized.

DavyLandman commented May 24, 2013

yes, I too would like it to be in the official release.

yes, everything is non-blocking (with the exception for the write in case
of intensively writing large pieces of data within one loop() invocation).

On Fri, May 24, 2013 at 5:30 PM, bilics notifications@github.com wrote:

Thanks for the reply Davy - I would really like to see this in the
official release.

Anyway, I will download this version and try to use it in my project now.

As far as I understand, even the initialize() / initialized() calls should
be non-blocking, correct ?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1240#issuecomment-18411951
.

@bilics

This comment has been minimized.

bilics commented May 24, 2013

Hi Davy,

I am facing an issue here:

For some reason, my arduino ethernet shield is not getting a response from the DHCP server.

Looking at the code, specifically at the method "DhcpClass::beginWithDHCP()", there is a "while (result == 0)", which blocks the Ethernet.initialize(mac) call in the case where there is no apparent response from a DHCP server.

In my case, the loop blocks the system at the state "STATE_DHCP_DISCOVER"(or at least until some timeout is reached).

Is this expected behaviour ? Even if there is a hardware problem in my arduino (or dhcp server) ?

Thanks again for the help,

@DavyLandman

This comment has been minimized.

DavyLandman commented May 24, 2013

Post your code in a gist, as a minimal example.

Also how did you use this version?
On May 24, 2013 9:27 PM, "bilics" notifications@github.com wrote:

Hi Davy,

I am facing an issue here:

For some reason, my arduino ethernet shield is not getting a response from
the DHCP server.

Looking at the code, specifically at the method
"DhcpClass::beginWithDHCP()", there is a "while (result == 0)", which
blocks the Ethernet.initialize(mac) call in the case where there is no
apparent response from a DHCP server.

In my case, the loop blocks the system at the state
"STATE_DHCP_DISCOVER"(or at least until some timeout is reached).

Is this expected behaviour ? Even if there is a hardware problem in my
arduino (or dhcp server) ?

Thanks again for the help,


Reply to this email directly or view it on GitHubhttps://github.com//pull/1240#issuecomment-18424917
.

@bilics

This comment has been minimized.

bilics commented May 24, 2013

I replaced the libraries/Ethernet directory of my installation (I am on a Mac, so "show package contents" for Arduino, replace files under libraries/Ethernet) with this one.

I just created a new project with the example code you posted above.

Is this the proper way to test it ?

In order to find out where it was blocking, I added "Serial.println()" to several of the ethernet calls and found out that the "while" inside "DhcpClass::beginWithDHCP()" is what is blocking the system.

Thanks again for the swift reply!

@DavyLandman

This comment has been minimized.

DavyLandman commented May 24, 2013

I will look into it later in this weekend, however what version of the
Arduino library (app) did you paste it in?
On May 24, 2013 9:36 PM, "bilics" notifications@github.com wrote:

I replaced the libraries/Ethernet directory of my installation (I am on a
Mac, so "show package contents" for Arduino, replace files under
libraries/Ethernet) with this one.

I just created a new project with the example code you posted above.

Is this the proper way to test it ?

In order to find out where it was blocking, I added "Serial.println()" to
several of the ethernet calls and found out that the "while" inside
"DhcpClass::beginWithDHCP()" is what is blocking the system.

Thanks again for the swift reply!


Reply to this email directly or view it on GitHubhttps://github.com//pull/1240#issuecomment-18425385
.

@bilics

This comment has been minimized.

bilics commented May 24, 2013

Thanks!

I am using version 1.0.4.

@bilics

This comment has been minimized.

bilics commented May 30, 2013

Hi there,

I am not sure if you received my reply: working with Arduino 1.0.4.

I tried using another non-blocking library available for DHCP:

http://gkaindl.com/software/arduino-ethernet/dhcp

But it does not work either.

Cheers

On Fri, May 24, 2013 at 4:56 PM, Davy Landman notifications@github.comwrote:

I will look into it later in this weekend, however what version of the
Arduino library (app) did you paste it in?
On May 24, 2013 9:36 PM, "bilics" notifications@github.com wrote:

I replaced the libraries/Ethernet directory of my installation (I am on
a
Mac, so "show package contents" for Arduino, replace files under
libraries/Ethernet) with this one.

I just created a new project with the example code you posted above.

Is this the proper way to test it ?

In order to find out where it was blocking, I added "Serial.println()"
to
several of the ethernet calls and found out that the "while" inside
"DhcpClass::beginWithDHCP()" is what is blocking the system.

Thanks again for the swift reply!


Reply to this email directly or view it on GitHub<
https://github.com/arduino/Arduino/pull/1240#issuecomment-18425385>
.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1240#issuecomment-18426488
.

@DavyLandman

This comment has been minimized.

DavyLandman commented May 31, 2013

Sorry, had some personal stuff, didn't forget.

If you try a the normal library, does dhcp even work? Could be a network
thing, I hope I can look into it this weekend, but when I developed the
noon blocking variant, I always used dhcp, it was actually the first thing
I made non blocking.
On May 31, 2013 12:07 AM, "bilics" notifications@github.com wrote:

Hi there,

I am not sure if you received my reply: working with Arduino 1.0.4.

I tried using another non-blocking library available for DHCP:

http://gkaindl.com/software/arduino-ethernet/dhcp

But it does not work either.

Cheers

On Fri, May 24, 2013 at 4:56 PM, Davy Landman notifications@github.comwrote:

I will look into it later in this weekend, however what version of the
Arduino library (app) did you paste it in?
On May 24, 2013 9:36 PM, "bilics" notifications@github.com wrote:

I replaced the libraries/Ethernet directory of my installation (I am
on
a
Mac, so "show package contents" for Arduino, replace files under
libraries/Ethernet) with this one.

I just created a new project with the example code you posted above.

Is this the proper way to test it ?

In order to find out where it was blocking, I added "Serial.println()"
to
several of the ethernet calls and found out that the "while" inside
"DhcpClass::beginWithDHCP()" is what is blocking the system.

Thanks again for the swift reply!


Reply to this email directly or view it on GitHub<
https://github.com/arduino/Arduino/pull/1240#issuecomment-18425385>
.


Reply to this email directly or view it on GitHub<
https://github.com/arduino/Arduino/pull/1240#issuecomment-18426488>
.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1240#issuecomment-18711618
.

@TrippyLighting

This comment has been minimized.

TrippyLighting commented Mar 7, 2015

Somehow from the provided example sketches I can't figure out how to use this library non-blocking.
The behavior I would like is very well described in the PollingDHCP.pde that can be found in the line provide above http://gkaindl.com/software/arduino-ethernet/dhcp
The code below does exactly what I would want and I've used it a long time ago (Arduino 0023 ?). The Ethernet.begin returns immediately and one can continue with the sketch. Then in the loop the state of the DHCP is being polled.

void setup()
{
  Serial.begin(9600);

  // Initiate a DHCP session. The argument is the MAC (hardware) address that
  // you want your Ethernet shield to use. The second argument enables polling
  // mode, which means that this call will not block like in the
  // SynchronousDHCP example, but will return immediately.
  // Within your loop(), you can then poll the DHCP library for its status,
  // finding out its state, so that you can tell when a lease has been
  // obtained. You can even find out when the library is in the process of
  // renewing your lease.
  EthernetDHCP.begin(mac, 1);
}

void loop()
{
  static DhcpState prevState = DhcpStateNone;
  static unsigned long prevTime = 0;

  // poll() queries the DHCP library for its current state (all possible values
  // are shown in the switch statement below). This way, you can find out if a
  // lease has been obtained or is in the process of being renewed, without
  // blocking your sketch. Therefore, you could display an error message or
  // something if a lease cannot be obtained within reasonable time.
  // Also, poll() will actually run the DHCP module, just like maintain(), so
  // you should call either of these two methods at least once within your
  // loop() section, or you risk losing your DHCP lease when it expires!
  DhcpState state = EthernetDHCP.poll();

  if (prevState != state) {
    Serial.println();

    switch (state) {
      case DhcpStateDiscovering:
        Serial.print("Discovering servers.");
        break;
      case DhcpStateRequesting:
        Serial.print("Requesting lease.");
        break;
      case DhcpStateRenewing:
        Serial.print("Renewing lease.");
        break;
      case DhcpStateLeased: {
        Serial.println("Obtained lease!");

        // Since we're here, it means that we now have a DHCP lease, so we
        // print out some information.
        const byte* ipAddr = EthernetDHCP.ipAddress();
        const byte* gatewayAddr = EthernetDHCP.gatewayIpAddress();
        const byte* dnsAddr = EthernetDHCP.dnsIpAddress();

        Serial.print("My IP address is ");
        Serial.println(ip_to_str(ipAddr));

        Serial.print("Gateway IP address is ");
        Serial.println(ip_to_str(gatewayAddr));

        Serial.print("DNS IP address is ");
        Serial.println(ip_to_str(dnsAddr));

        Serial.println();

        break;
      }
    }
  } else if (state != DhcpStateLeased && millis() - prevTime > 300) {
     prevTime = millis();
     Serial.print('.'); 
  }

  prevState = state;
}

// Just a utility function to nicely format an IP address.
const char* ip_to_str(const uint8_t* ipAddr)
{
  static char buf[16];
  sprintf(buf, "%d.%d.%d.%d\0", ipAddr[0], ipAddr[1], ipAddr[2], ipAddr[3]);
  return buf;
}
@DavyLandman

This comment has been minimized.

DavyLandman commented Mar 9, 2015

Hi @TrippyLighting ,

I made the pull-request over 2 years ago. Back then, it was working nicely, with perhaps a bit of polishing needed on the API choices.

However, perhaps @cmaglie should just close the pull-request, since I would be surprised if the past 2 years haven't seen enough evolution to make the diffs almost useless.

The work on making it non-blocking was not trivial, and quite useful for me at the time, but I haven't done any Arduino development since then, and won't expect in the coming future.

@TrippyLighting

This comment has been minimized.

TrippyLighting commented Mar 9, 2015

Hi @DavyLandman ,

Thanks for the reply. Actually there are not that many things different now compared to the standard Ethernet library. I believe your additions would be well worth to be integrated into the standard Arduino Ethernet library. Alas the Arduino teams not known for their agility in integrating changes.

I've been looking through the code over the weekend and I think I understand now how this works. I've started mergin it with the Ethernet library that comes with Teensyduino, which is the Arduino IDE plugin that goes along with the Teensy boards from PJRC ( http://pjrc.com/teensy/index.html ) .

I know what you mean with non-trivial. Last year I did a complete overhaul of the Arduino Bonjour library to rely only on UDP calls instead of talking directly to he W5100/5200 chip. That was a heck of an undertaking given my level of programming abilities. Works nice now though.

Thanks for your work. I'll let you know how things work out!

@PaulStoffregen

This comment has been minimized.

Collaborator

PaulStoffregen commented Mar 9, 2015

FWIW, I've been holding off UDP multicast and a number of other Ethernet changes, until Arduino 1.6.0.... which is now released.

Before 1.6.0, Teensyduino had to overwrite the Ethernet library. I was reluctant to do too much with it, since that meant changing it for all other boards. Now it'll be possible to add this stuff, without unexpectedly changing the copy used for other boards.

At the moment, I'm crazy busy finalizing version 1.21 and releasing the new LC board. But soon I'm going to look at this and lots of other things that just weren't feasible to do before 1.6.0's better library management.

I can't speak for what the Arduino Team is willing to accept & merge. But historically, after stuff has been successfully used on Teensy for a year or more, fears over compatibility or unexpected problems unusually ease.

@PaulStoffregen

This comment has been minimized.

Collaborator

PaulStoffregen commented Mar 9, 2015

Long-term, I have some very ambitious plans for Ethernet to use the W5200 interrupt signal and support special "server" libraries. A lot of that work probably won't ever be feasible on memory-limited AVR. But on ARM Cortex-M chips, we ought to be able to create libraries that act like daemons on Linux or Unix systems, and do almost all their work using nested priority interrupts, and provide nice Arduino-style APIs to the sketch code for configuration and status.

@TrippyLighting

This comment has been minimized.

TrippyLighting commented Mar 11, 2015

I am looking over the example sketches, but these seem to be unchanged from the originals and do not work.
"if (Ethernet.begin()==0)" does not compile (using a Teensy 3.0 Arm based board) because in Ethernet.h the return value is "void" and not "int".

Is there any example of how this is supposed to work. Do I need to call Ethernet.maintain() in loop() and respond to it's return value ?

@TrippyLighting

This comment has been minimized.

TrippyLighting commented Mar 13, 2015

After upgrading the NBEthernet library to use the new SPI transactions it works nicely.
If it needs to work with Teensyduino then the contents of the /utility folder also have to be replaced with the equivalent files from Teensyduino.

boolean _ethernetInitialized = 0;

setup(){
  Serial.begin(9600;
  Ethernet.begin(myMac);
}

loop(){
  if(Ethernet.initialized()){
    if( Ethernet.maintain()==0){
      if (_ethernetInitialized == 0){
        Serial.print("My IP is ");
        Serial.println(Ethernet.localIP());
        ethernetInitialized = 1;
      }
    }
  }
}

@PaulStoffregen PaulStoffregen force-pushed the arduino:master branch from 57dcf07 to 5a68e34 May 19, 2015

@bennigraf

This comment has been minimized.

bennigraf commented Oct 26, 2015

@TrippyLighting,

I understand you use the NBEthernet library with a teensy? I'm trying the same thing but I seem to install it wrong. Where did you place the files and what did you #include?

I'd love to see non-blocking ethernet in Arduino core as well, btw!

Best, Benni.

@DavyLandman

This comment has been minimized.

DavyLandman commented Oct 26, 2015

fyi, the NBEthernet library was abandoned, I started with this pull request
since it would take too much cloning.

On Mon, 26 Oct 2015 at 14:58 Benjamin Graf notifications@github.com wrote:

@TrippyLighting https://github.com/TrippyLighting,

I understand you use the NBEthernet library with a teensy? I'm trying the
same thing but I seem to install it wrong. Where did you place the files
and what did you #include?

I'd love to see non-blocking ethernet in Arduino core as well, btw!

Best, Benni.


Reply to this email directly or view it on GitHub
#1240 (comment).

@TrippyLighting

This comment has been minimized.

TrippyLighting commented Oct 26, 2015

@bennigraf
You are correct. However, you cannot just download the version provided here by Davy. Some stuff has changed in the since Davy released it, for example the Ethernet library - particularly the one that ships with Teesnyduino - now uses SPI transactions.
I updated this NBEtherent library to work with SPI transactions and it works nicely. In my initial search I created a thread on the Teensy Forum:
https://forum.pjrc.com/threads/27980-Non-Blocking-Ethernet-DHCP-library-(-)?highlight=blocking+Ethernet
Please post there as a reminder for me and I'll post a zipped file containing the updated library.

tommag added a commit to tommag/Ethernet that referenced this pull request May 26, 2017

@tommag tommag referenced this pull request May 26, 2017

Open

Non-blocking DHCP #13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment