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 copy assignment operator #16

Closed

Conversation

turkycat
Copy link
Contributor

This PR resolves issue #15

NOTE: This pull request will show "Able to merge", as my commit is added on top of the current master branch. However, this will cause one small conflict with #14

I suggest pulling #14 and #13 (which I have tested), and allow me to make the necessary changes to my existing commit before pulling this request!

@turkycat
Copy link
Contributor Author

I have rebased these changes on top of the newly-updated master branch, this is ready to merge.

@sandeepmistry
Copy link
Contributor

@turkycat thanks for submitting!

A few notes:

  1. The new equal's operator is missing a return statement (*this).

  2. I'm still having issues with the follow sketch. Available is still returning 0, with your patch. I am looking at what the issue is.

#include <WiFi101.h>

char ssid[] = "yourNetwork";     //  your network SSID (name)
char pass[] = "secretPassword";  // your network password
int status = WL_IDLE_STATUS;     // the Wifi radio's status

WiFiClient client;  //stack memory allocated, default ctor invoked
WiFiServer server(23);

void setup() {
  //Initialize serial and wait for port to open:
  Serial.begin(9600);
  while (!Serial) {
    ; // wait for serial port to connect. Needed for native USB port only
  }

  // check for the presence of the shield:
  if (WiFi.status() == WL_NO_SHIELD) {
    Serial.println("WiFi shield not present");
    // don't continue:
    while (true);
  }

  // attempt to connect to Wifi network:
  while ( status != WL_CONNECTED) {
    Serial.print("Attempting to connect to WPA SSID: ");
    Serial.println(ssid);
    // Connect to WPA/WPA2 network:
    status = WiFi.begin(ssid, pass);
  }

  // you're connected now, so print out the data:
  Serial.println("You're connected to the network");

  IPAddress ip = WiFi.localIP();
  Serial.print("IP Address: ");
  Serial.println(ip);

  server.begin();
}

void loop() {
  client = server.available();
  if (client) {
    if ( client.available() ) {
      char c = client.read();
      Serial.print(c);
    }
  }
}

@sandeepmistry
Copy link
Contributor

Things are a little better, when I add a check to setMembersAndWiFiCache, to guard against a socket value of -1. However, the sketch sometimes prints out repeated or invalid data.

void WiFiClient::setMembersAndWiFiCache(const WiFiClient& other)
{
    _socket = other._socket;
    _flag = other._flag;
    _head = other._head;
    _tail = other._tail;
    for (int sock = 0; sock < TCP_SOCK_MAX; sock++) {
        if (WiFi._client[sock] == this)
            WiFi._client[sock] = 0;
    }

    if (_socket > -1) {
        WiFi._client[_socket] = this;

        // Add socket buffer handler:
        socketBufferRegister(_socket, &_flag, &_head, &_tail, (uint8 *)_buffer);

        // Enable receive buffer:
        recv(_socket, _buffer, SOCKET_BUFFER_MTU, 0);
    }

    m2m_wifi_handle_events(NULL);
}

@sandeepmistry
Copy link
Contributor

I forgot to mention this before, without the _socket > -1 check, socketBufferRegister would "crash" my Zero board.

I've added a memcpy to WiFiClient::setMembersAndWiFiCache and am no longer seeing the repeated/invalid data mentioned previously. This can be optimized to avoid copying the whole buffer size.

void WiFiClient::setMembersAndWiFiCache(const WiFiClient& other)
{
    _socket = other._socket;
    _flag = other._flag;
    _head = other._head;
    _tail = other._tail;
    memcpy(_buffer, other._buffer, sizeof(_buffer));
    for (int sock = 0; sock < TCP_SOCK_MAX; sock++) {
        if (WiFi._client[sock] == this)
            WiFi._client[sock] = 0;
    }

    if (_socket > -1) {
        WiFi._client[_socket] = this;

        // Add socket buffer handler:
        socketBufferRegister(_socket, &_flag, &_head, &_tail, (uint8 *)_buffer);

        // Enable receive buffer:
        recv(_socket, _buffer, SOCKET_BUFFER_MTU, 0);
    }

    m2m_wifi_handle_events(NULL);
}

(It's a bigger change, but maybe it's worth considering keeping the socket buffer outside of WiFiClient in the future. This would avoid copying and re-registering the socket buffers.)

@sandeepmistry
Copy link
Contributor

@turkycat I've pushed a branch based on this pull request with the changes discussed above:

https://github.com/sandeepmistry/WiFi101/tree/copy-assignment-operator

@sandeepmistry
Copy link
Contributor

One more changed pushed to my branch, I've renamed setMembersAndWiFiCache to copyFrom, and also made it private:

sandeepmistry@634e77f

@turkycat
Copy link
Contributor Author

turkycat commented Jan 4, 2016

Ahh, yes it definitely should have been private. I like the name change as well. I'll try out the changes with the tests I've been running as well as your sample sketch above and will report back

@turkycat
Copy link
Contributor Author

turkycat commented Jan 4, 2016

I was able to take your changes and run both your sketch above as well as my full Firmata implementation that uses WiFi101 within a Stream wrapper class... all with no issues

@sandeepmistry
Copy link
Contributor

Great thanks for trying the changes out!

I've submitted #20 which includes the changes included in this PR.

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.

None yet

2 participants