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

WiFiStream variant integration #285

Merged
merged 0 commits into from
Apr 11, 2016
Merged

WiFiStream variant integration #285

merged 0 commits into from
Apr 11, 2016

Conversation

jnsbyr
Copy link
Contributor

@jnsbyr jnsbyr commented Apr 9, 2016

STATIC_IP_ADDRESS is currently not commented out for testing purposes in wifiConfig.h

@@ -140,11 +153,11 @@ char wep_key[] = "your_wep_key";
* CONFIGURATION ERROR CHECK (don't change anything here)
*============================================================================*/

#if ((defined(ARDUINO_WIFI_SHIELD) && (defined(WIFI_101) || defined(HUZZAH_WIFI))) || (defined(WIFI_101) && defined(HUZZAH_WIFI)) || (defined(WIFI_101) && defined(ESP_WIFI)) || (defined(ESP_WIFI) && defined(HUZZAH_WIFI)) || (defined(ESP_WIFI) && defined(ARDUINO_WIFI_SHIELD)))
#if ((defined(ARDUINO_WIFI_SHIELD) && (defined(WIFI_101) || defined(HUZZAH_WIFI))) || (defined(WIFI_101) && defined(HUZZAH_WIFI)) || (defined(WIFI_101) && defined(ESP8266_WIFI)) || (defined(ESP8266_WIFI) && defined(HUZZAH_WIFI)) || (defined(ESP8266_WIFI) && defined(ARDUINO_WIFI_SHIELD)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to figure out a more scalable way to handle this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For ARDUINO_WIFI_SHIELD and WIFI_101 one could provide the two include alternatives and use only one define.

I assume the HUZZAH_WIFI define is reserved for the CC3000 WiFi shield. I assume it comes with its own library, but I don't know if it is identical with WiFi/WiFi101. The HUZZAH ESP8266 is covered by the ESP8266.

With the ESP8266 one could use the architecture define already in use in Boards.h. This could replace the ESP_WIFI define and could automatically exclude all shields.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one hacky way to do it, would be for each Wi-Fi driver definition, increment a counter (wifiDriverCount). Initialize it to 0. If no drivers are included wifiDriverCount = 0, throw an error. If more than 1 driver is included (wifiDriverCount > 1), throw an error.

@soundanalogous
Copy link
Member

This script can be used to test the Wi-Fi server implementation: https://gist.github.com/soundanalogous/2fbeb29c39f11498fd7f

I'll check with Arduino 101 shield and Arduino MKR1000 later today.

@@ -822,37 +832,37 @@ void systemResetCallback()
}

void printWifiStatus() {
#if defined(ARDUINO_WIFI_SHIELD) || defined(WIFI_101)
#if defined(ARDUINO_WIFI_SHIELD) || defined(WIFI_101) || defined(ESP8266_WIFI)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think any of these #ifdefs are needed anymore since all supported libraries support the following function calls. If a lib does not support one of these calls in the future, we could use an #ifndef instead. I really want to try to reduce the number of conditional compiler statements in the StandardFirmata variants.

@soundanalogous
Copy link
Member

I'm not having any luck with the Adafruit Huzzah ESP8266 board. I noticed there are some differences between the ESP8266 WiFi API and the Arduino WiFi api, such as begin does not return the state in the ESP8266 api (so this will not return anything). I tried making some changes in WiFiStream such as using WiFi.status() to reflect this but no luck. Seems anything I try hangs the board while it's trying to connect via WPA.

@soundanalogous
Copy link
Member

I was able to get a WiFi client version with the ESP8266 simply using EthernetClientStream, inspired by this Fork: kotl@cfeb9c4.

Here is a test sketch file
And the accompanying config file

@jnsbyr
Copy link
Contributor Author

jnsbyr commented Apr 10, 2016

I hadn't made any technical changes jet required for the ESP8266 variant.

Using EthernetClientStream is what I referred to here. But you still need to use WiFi.begin() and WiFi.status() and this is also used in WiFiStream. I had no problem using WiFi.status() (see below).

This is what I changed in StandardFirmataWiFi to make it work using HUZZAH_WIFI as switch and what I currently use with my Huzzah ESP8266. :

#ifdef HUZZAH_WIFI
#include <ESP8266WiFi.h>
#include "utility/EthernetClientStream.h"
WiFiClient client;

#ifdef SERVER_IP
#ifdef STATIC_IP_ADDRESS
EthernetClientStream stream(client, IPAddress(STATIC_IP_ADDRESS), IPAddress(SERVER_IP), NULL, SERVER_PORT);
#else
EthernetClientStream stream(client, IPAddress(0, 0, 0, 0), IPAddress(SERVER_IP), NULL, SERVER_PORT);
#endif
#endif

...

#ifdef HUZZAH_WIFI  
  WiFi.config(IPAddress(STATIC_IP_ADDRESS), IPAddress(NETWORK), IPAddress(GATEWAY));
#else  
  stream.config(STATIC_IP_ADDRESS);
#endif

...

#elif defined(WIFI_WPA_SECURITY)
  while (wifiStatus != WL_CONNECTED) {
    DEBUG_PRINT( "Attempting to connect to WPA SSID: " );
    DEBUG_PRINTLN(ssid);
#if defined(HUZZAH_WIFI)    
    WiFi.begin(ssid, wpa_passphrase);
    wifiStatus = WiFi.status();
#else
    wifiStatus = stream.begin(ssid, wpa_passphrase, SERVER_PORT);
#endif        
    delay(5000); // TODO - determine minimum delay
    if (++wifiConnectionAttemptCounter > WIFI_MAX_CONN_ATTEMPTS) break;
  }

...

#if defined(HUZZAH_WIFI)    
  stream.maintain(IPAddress(STATIC_IP_ADDRESS));
#else
  stream.maintain();
#endif  

But this results in a lot of defines in the ino file. My idea is to use a base class WiFiStream with the common code for WiFi, client and server and a WiFiServerStream and a WiFiClientStream class that inherit from WiFiStream. This should reduce or at least hide the number of defines and would avoid mixing everything in one class with a lot of ifs for the client and server specials.

@jnsbyr
Copy link
Contributor Author

jnsbyr commented Apr 10, 2016

Had another look at kotl/esp8266-firmata@cfeb9c4. Although not a default approach for Firmata but found in several IT devices the serial port is used to configure the network parameters. Another way to do it that is to define a protocol extension for the setup parameters and to start Firmata with invalid EEPROM data in server mode using DHCP. After configuration Firmata keeps its WiFi secrets and network parameters in EEPROM and may even switch to client mode. That could be an interesting project, but first things first:

How should we proceed with the client/server integration? Separate INOs and/or separate classes with or without inheritance? As soon as that is decided, I could implement the client, test with the Huzzah ESP8266 and make the necessary changes for the server and the INO.

@soundanalogous
Copy link
Member

We could rename EthernetClientStream to NetworkClientStream since it works for both Ethernet and WiFi. However doing something similar for WiFiStream is tricky since I'd Ideally want an Ethernet server variant as well and unlike EthernetClientStream which makes no reference to the Ethernet or WiFi libraries, WiFiStream references the WiFi library.

Were you able to get WiFi server working on ESP8266 or only WiFi client?

@soundanalogous
Copy link
Member

This PR is also starting to get fragmented. It's partly about ESP8266 integration and partly about merging WiFiStream and WiFi101Stream and partly about creating a separate WiFi server and WiFi client implementation. Should probably be 3 different PRs or we'll end up with a rats nest.

@soundanalogous
Copy link
Member

I think we should proceed as follows:

  1. Cherry pick commits that added ESP8266 to Boards.h to master (I can alternately merge esp in it's current state to get exactly that + the DEFAULT_PWM_RESOLUTION change).
  2. Resolve WiFiStream and WiFi101Stream into a single WiFiStream class and merge to master
  3. Figure out the best approach to having WiFi server and client support and work on this in a new branch
  4. ESP8266 WiFi integration
  5. Ethernet server

@jnsbyr
Copy link
Contributor Author

jnsbyr commented Apr 10, 2016

  1. Go ahead by merging esp Boards.h into master.
  2. This needs removing one include from WiFiStream.h, deleting WiFi101Stream and adding two includes to the wifiConfig.h.
  3. Please create this branch, but I would prefer to have the ESP8266 along. Because of the slightly different requirements it could help find the best solution for the client/server support. Having ESP8266 on board from the beginning would also allow me to do tests with my hardware.

@jnsbyr
Copy link
Contributor Author

jnsbyr commented Apr 10, 2016

Were you able to get WiFi server working on ESP8266 or only WiFi client?

I have the WiFiClient working in the configuration mentioned above, but I haven't tried anything with this PR or a WiFiServer so far.

I already considered reusing the EthernetClientStream because its working, but I still think WiFi.maintain() should be basically the same for client and server so I suggest a WiFiStream/WiFiClientStream approach instead of sharing EthernetClientStream.

@soundanalogous
Copy link
Member

I have discovered what the issue is with ESP8266 WiFi server. In WiFiStream, this line returns 0 if a connection is established. I suspect this is to enable the user to establish a new connection with a different access point. This works fine with the Arduino WiFi library as this line always seems to return the connected state with Arduino WiFi (I assume it waits) whereas it never does with ESP8266WiFi (I assume it does not wait). What happens then is with ESP8266 the connection status is updated later, but is never returned since a connected state upon calling begin always returns false.

I'm looking into solutions that don't involve adding additional #ifdefs to the sketch file but it has proven to be a bit tricky to find such a solution.

@soundanalogous
Copy link
Member

Maybe I could set a variable is_new_connection to true each time WiFi.begin is called and then change the following:

 if( !is_ready() ) return 0;

to something like:

if (is_new_connection && WiFi.status() == WL_CONNECTED) {
  is_new_connection = false;
  return WL_CONNECTED;
}
if (!is_ready()) return 0;

Something like that would be more stable for both ESP8266 and Arduino WiFi implementations. But it would create a lot of repeated code... maybe refactoring the is_ready() function would be better

@jnsbyr
Copy link
Contributor Author

jnsbyr commented Apr 11, 2016

I assume it does not wait

That is very probable. With the ESP8266 waiting is not a good idea. After a max. of 1 second the watchdog will trigger. Everything that intends to wait must periodically exit the main loop or call yield().

From my point of view a WiFi TCP connections differs from Ethernet TCP connection becaus it needs two connects. First there is the connection from the WiFi station to the AP and then there is the connection from the TCP client to the server. They should be handled separately and hierarchically. As long as there is no WiFi link one does not need to bother with the TCP link. Maybe using WiFi.status() is the common ground of Arduino and ESP8266. Establishing the WiFi link and the TCP connection should be performed non-blocking if possible, at least when reconnecting, better always.

Let begin() just do the initialisation and then maintain() can do the rest.

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.

2 participants